This repository has been archived by the owner on Jul 12, 2020. It is now read-only.
Fix seamless panel caret #88
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The caret is combined with the header content. It is usually rendered
like this:
> Some heading
However, custom header contents, after markdown rendering, will be
wrapped with block HTML tags such as <p> and <h1>, which will make
browsers render a line break between the caret and header content:
>
Some heading
Let's fix this by moving the caret to a separate div. Then, make
everything inside a panel header be on a single line by using CSS
'display: inline-block' and 'width: ...', such that the width adds up to
100%.
+--------+-----------------+---------+
| caret | header-content | buttons |
| (32px) | (100% - 32 - 32)| (32px) |
+--------+-----------------+---------+
Thanks to @nusjzx for providing the fix to the button-wrapper in PR
MarkBind#86, in which his use of
'calc(100% - 32px)' provided a source of inspiration for this fix. His
fix in that PR is incorporated into this commit because without it, the
responsive design will not work.
An alternative fix considered was to use md.renderInline(). However,
md.renderInline() does not render markdown headings.
Another alternative fix was to insert the caret directly inside the
header content. However, after PR MarkBind#74, MarkBind#81 and MarkBind#83 were merged, new
problems continue to emerge.
These problems are reported as comments in these pages:
- MarkBind#74
- MarkBind#81
- MarkBind#83
- MarkBind/markbind#373
- MarkBind/markbind#383
As such, the PRs for this alternative fix has been reverted by the
previous commits before this commit.
src/Panel.vue
Outdated
| @@ -16,7 +16,10 @@ | |||
| <div :class="['card-header',{'header-toggle':isExpandableCard}, cardType, borderType]" | |||
| @click.prevent.stop="isExpandableCard && toggle()" | |||
| @mouseover="onHeaderHover = true" @mouseleave="onHeaderHover = false"> | |||
| <div class="header-wrapper" ref="headerWrapper"> | |||
| <div class="caret-wrapper"> | |||
| <span :class="['glyphicon', localExpanded ? 'glyphicon-chevron-down' : 'glyphicon-chevron-right']" v-show="showCaret"></span> | |||
There was a problem hiding this comment.
v-if is preferred since it doesn't need to be rendered.
src/Panel.vue
Outdated
| @@ -27,12 +30,12 @@ | |||
| @click.native.stop.prevent="expand()" | |||
| @is-open-event="retrieveOnOpen" :is-light-bg="isLightBg"></panel-switch> | |||
| <button type="button" :class="['close-button', 'btn', isLightBg ? 'btn-outline-secondary' : 'btn-outline-light']" | |||
| v-show="isSeamless ? onHeaderHover : (!noCloseBool)" | |||
| v-show="!isSeamless ? (!noCloseBool) : onHeaderHover" | |||
src/Panel.vue
Outdated
| @click.stop="close()"> | ||
| <span class="glyphicon glyphicon-remove" aria-hidden="true"></span> | ||
| </button> | ||
| <button type="button" :class="['popup-button', 'btn', isLightBg ? 'btn-outline-secondary' : 'btn-outline-light']" | ||
| v-show="((this.popupUrl !== null) && (!isSeamless || onHeaderHover))" |
This change will improve rendering performance, as `v-if` will ensure that the caret will never get rendered if it shouldn't appear in the first place.
For seamless panels, header buttons should only show when the user hovers over the title. However, the popup button is always shown to the user regardless of whether the user hovers over it or not. Let's fix the popup button showing behavior so that it will only show when the user hovers over it.
|
Update:
|
Chng-Zhi-Xuan
approved these changes
Jul 30, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [X] Bug fix
Fixes MarkBind/markbind#383, fixes MarkBind/markbind#373.
What is the rationale for this request?
Fix the seamless panel caret once and for all, by just moving the caret outside the panel header.
What changes did you make? (Give an overview)
Since PR #81, carets were inserted inside the header. However, the algorithm for inserting the caret is not perfect, and there were many edge cases. This has lead to many issues as reported as comments in #74
, #81, #83, MarkBind/markbind#373, MarkBind/markbind#383.
Those fixes were unsustainable. Therefore, the relevant PRs have been reverted, and this PR tries a new fix for the original issue.
Provide some example code that this change will affect:
NA
Is there anything you'd like reviewers to focus on?
Ensuring that the panel rendering did not break.
Testing instructions:
vue-strap.min.jsand copy to MarkBind repository.index.md: