Skip to content
This repository was archived by the owner on Jul 12, 2020. It is now read-only.

Conversation

@nusjzx
Copy link

@nusjzx nusjzx commented Jul 26, 2018

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

• [ ] Documentation update
• [X] Bug fix
• [ ] New feature
• [ ] Enhancement to an existing feature
• [ ] Other, please explain:

Fixes MarkBind/markbind#370

What is the rationale for this request?
Current: headings overflow to next line although there seems to be enough space to keep them in one line. Should not use percentage to specify the width, as button has fixed width.

What changes did you make? (Give an overview)
change width of header wrapper to 100% - button width

Testing instructions:

  1. npm run build copy paste vue-strap.min.js
  2. check 2103 website week3.1b heading not overflow

.header-wrapper {
display: inline-block;
width: 72%;
width: calc(100% - 32px);
Copy link
Member

@yamgent yamgent Jul 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32px is too small, one button is already 28px.

A panel can has as many as 3 buttons.

<panel header="This is a header" popup-url="https://www.yahoo.com">
This is some content.
</panel>

EDIT: @media (max-width: 575.98px) { also has a .header-wrapper as well, and that one can use 32px because they will all align in one vertical line.

yamgent added a commit to yamgent/vue-strap that referenced this pull request Jul 27, 2018
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.
yamgent added a commit to yamgent/vue-strap that referenced this pull request Jul 27, 2018
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 added a commit to yamgent/vue-strap that referenced this pull request Jul 27, 2018
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
Copy link
Member

yamgent commented Jul 27, 2018

Superseded and incorporated by #88.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panels: allow more space for panel heading

2 participants