Enable seamless Panel's caret to stay inline for custom header slots #83

yamgent · 2018-07-23T07:51:27Z

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [X] Enhancement to an existing feature

What is the rationale for this request?
Previously, on #74, we enable seamless Panel's caret to stay inline.

However, that logic only works if it is not using custom header slots, so the caret will break for custom cases, like in here. This can be very irritating for the author.

What changes did you make? (Give an overview)
This PR will fix the problem mentioned above, by implementing the logic for custom header slots.

Provide some example code that this change will affect:
NIL

Is there anything you'd like reviewers to focus on?
NIL

Testing instructions:

Run npm run build, and copy vue-strap.min.js to MarkBind repository.
Create a new MarkBind website with the following code in index.md:

<panel type="seamless">
<span slot="header" class="card-title">
This header won't break even if I used <code>card-title</code>, hooray!
</span>
</panel>

<panel type="seamless">
<span slot="header">
No breaking as <code>card-title</code> not used, hip hip hooray!
</span>
</panel>

<panel type="seamless" header="Normal seamless">
Normal seamless panel content.
</panel>

Run markbind serve. Ensure that all the panel headings are inline even when the window is resized to be very small.

Chng-Zhi-Xuan

Tested and works well for even <p> tags with slot attribute. 👍

Chng-Zhi-Xuan · 2018-07-23T08:02:55Z

src/Panel.vue

+          const wrappedElementName = jQuery(headerInnerHTML).attr("name");
+          this.$refs.headerWrapper.innerHTML =
+            jQuery(headerInnerHTML).unwrap().prepend(this.caretHtml + ' ')
+              .wrap(`<${wrappedElementName}></${wrappedElementName}>`).parent().html();


Should abstract this to a method for code quality and maintainability. Maybe insertCaretInsideCustomHeader?

yamgent · 2018-07-23T08:26:33Z

Update:

Changes requested made.

damithc · 2018-07-25T08:28:42Z

Looks like the problem moved to a different place :-)

The code looks like this (in chapter.njk):

<panel type="seamless" expanded>
  <span slot="header" class="card-title"><include src="{{ level_location }}/text.md#title" /></span>

…ret" This reverts commit 576952c, reversing changes made to 1ce9ce3.

@nusjzx

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. Therefore, this commit shall be viewed as the canonical fix for issue MarkBind/markbind#337.

@nusjzx

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.

@nusjzx

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.

yamgent requested a review from Chng-Zhi-Xuan July 23, 2018 07:51

Chng-Zhi-Xuan reviewed Jul 23, 2018

View reviewed changes

yamgent force-pushed the seamless-panel-caret branch 2 times, most recently from 76026af to bb4cf67 Compare July 23, 2018 08:20

yamgent added 3 commits July 23, 2018 16:23

Panel: Refactor logic for inserting caret inside custom header

c476439

Panel: Add caret insertion logic for custom header slots

9a908c6

Panel: Handle case when this.header is empty for renderedHeader()

11e6bee

yamgent force-pushed the seamless-panel-caret branch from bb4cf67 to 11e6bee Compare July 23, 2018 08:23

Chng-Zhi-Xuan approved these changes Jul 23, 2018

View reviewed changes

yamgent added this to the v2.0.1-markbind.16 milestone Jul 24, 2018

yamgent merged commit 576952c into MarkBind:master Jul 24, 2018

yamgent deleted the seamless-panel-caret branch July 24, 2018 03:45

yamgent mentioned this pull request Jul 25, 2018

Seamless panel: caret breaks if custom header slot is included from another file MarkBind/markbind#373

Closed

yamgent added a commit to yamgent/vue-strap that referenced this pull request Jul 27, 2018

Revert "Merge pull request MarkBind#83 from yamgent/seamless-panel-ca…

0f219f4

…ret" This reverts commit 576952c, reversing changes made to 1ce9ce3.

yamgent mentioned this pull request Jul 27, 2018

Fix seamless panel caret #88

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enable seamless Panel's caret to stay inline for custom header slots #83

Enable seamless Panel's caret to stay inline for custom header slots #83

Uh oh!

yamgent commented Jul 23, 2018

Uh oh!

Chng-Zhi-Xuan left a comment

Uh oh!

Chng-Zhi-Xuan Jul 23, 2018

Uh oh!

yamgent commented Jul 23, 2018 •

edited

Loading

Uh oh!

damithc commented Jul 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Enable seamless Panel's caret to stay inline for custom header slots #83

Enable seamless Panel's caret to stay inline for custom header slots #83

Uh oh!

Conversation

yamgent commented Jul 23, 2018

Uh oh!

Chng-Zhi-Xuan left a comment

Choose a reason for hiding this comment

Uh oh!

Chng-Zhi-Xuan Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

yamgent commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

damithc commented Jul 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yamgent commented Jul 23, 2018 •

edited

Loading