Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore merge-main@7f35bc5 #1167

Merged
merged 60 commits into from
Mar 31, 2022
Merged

Chore merge-main@7f35bc5 #1167

merged 60 commits into from
Mar 31, 2022

Conversation

@julien-deramond julien-deramond added v5 merge Sync with Bootstrap labels Mar 28, 2022
}

snippetButtonTooltip('.btn-clipboard', 'Copy to clipboard')
snippetButtonTooltip('.btn-edit', 'Edit on Stackblitz')
Copy link
Member Author

@julien-deramond julien-deramond Mar 29, 2022

Choose a reason for hiding this comment

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

Should be StackBlitz.
See twbs/bootstrap#36073

This comment was marked as outdated.

@@ -473,6 +473,7 @@ $utilities: map-merge(
lighter: $font-weight-lighter,
normal: $font-weight-normal,
bold: $font-weight-bold,
semibold: $font-weight-semibold,
Copy link
Member Author

@julien-deramond julien-deramond Mar 29, 2022

Choose a reason for hiding this comment

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

Check in Bootstrap if fw-semibold wasn't defined for examples. Otherwise remove it in Boosted.
See twbs/bootstrap#36075

- **Thicker table dividers are now opt-in.** We've removed the thicker and more difficult to override border between table groups and moved it to an optional class you can apply, `.table-group-divider`. [See the table docs for an example.]({{< docsref "/content/tables#table-group-dividers" >}})
- Added `.form-check-reverse` modifier to flip the order of labels and associated checkboxes/radios.

- Added striped columns support to tables via
Copy link
Member Author

@julien-deramond julien-deramond Mar 30, 2022

Choose a reason for hiding this comment

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

The end of the sentence seems to be missing.
See https://github.com/twbs/bootstrap/pull/36038/files#r839351166.

@julien-deramond julien-deramond marked this pull request as ready for review March 30, 2022 07:26
@julien-deramond
Copy link
Member Author

julien-deramond commented Mar 31, 2022

Tracked issues with StackBlitz in twbs/bootstrap#36088 and twbs/bootstrap#36091.

  • After the feedback (and merge?) of this PR, we need to not to forget checking Boosted custom parts of the documentation and execution in StackBlitz (same listing work than in the PR)

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

There are so much good points that were seen here, I can't even count them. But :

@@ -460,11 +462,11 @@ $body-text-align: null !default;
//
// Style anchor elements.

$link-color: var(--#{$boosted-variable-prefix}link-color, $black) !default; // Boosted mod
$link-color: $black !default; // Boosted mod
Copy link
Member

Choose a reason for hiding this comment

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

All the variables that were using $link-color should be replaced by var(--#{$prefix}link-color) to have the same result.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it everywhere via 8f4315b. It fixes the .btn-link issue for the dark variant.
Need to check but IMO Bootstrap should probably do the same.

Copy link
Member

Choose a reason for hiding this comment

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

scss/_variables.scss Show resolved Hide resolved
scss/_variables.scss Show resolved Hide resolved
scss/_navbar.scss Outdated Show resolved Hide resolved
color: $black;
background-color: $primary;
border-color: $primary;
&.active {
Copy link
Member

Choose a reason for hiding this comment

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

Be careful here, :active to .active -> break the pressed state + maybe reset the previous states

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried to fix it via b465249.

How to test all the cases:

{{< example >}}
<nav aria-label="Page navigation example">
  <ul class="pagination">
    <li class="page-item">
      <a class="page-link">Previous</a>
    </li>
    <li class="page-item">
      <a class="page-link" href="#">1</a>
    </li>
    <li class="page-item" aria-current="page">
      <a class="page-link" href="#">2</a>
    </li>
    <li class="page-item">
      <a class="page-link" href="#">3</a>
    </li>
    <li class="page-item">
      <a class="page-link" href="#">Next</a>
    </li>
  </ul>
  <ul class="pagination">
    <li class="page-item active">
      <a class="page-link">Previous</a>
    </li>
    <li class="page-item active">
      <a class="page-link" href="#">1</a>
    </li>
    <li class="page-item active" aria-current="page">
      <a class="page-link" href="#">2</a>
    </li>
    <li class="page-item active">
      <a class="page-link" href="#">3</a>
    </li>
    <li class="page-item active">
      <a class="page-link" href="#">Next</a>
    </li>
  </ul>
  <ul class="pagination">
    <li class="page-item">
      <a class="page-link active">Previous</a>
    </li>
    <li class="page-item">
      <a class="page-link active" href="#">1</a>
    </li>
    <li class="page-item" aria-current="page">
      <a class="page-link active" href="#">2</a>
    </li>
    <li class="page-item">
      <a class="page-link active" href="#">3</a>
    </li>
    <li class="page-item">
      <a class="page-link active" href="#">Next</a>
    </li>
  </ul>
  <ul class="pagination">
    <li class="page-item disabled">
      <a class="page-link">Previous</a>
    </li>
    <li class="page-item disabled">
      <a class="page-link" href="#">1</a>
    </li>
    <li class="page-item disabled" aria-current="page">
      <a class="page-link" href="#">2</a>
    </li>
    <li class="page-item disabled">
      <a class="page-link" href="#">3</a>
    </li>
    <li class="page-item disabled">
      <a class="page-link" href="#">Next</a>
    </li>
  </ul>
  <ul class="pagination">
    <li class="page-item">
      <a class="page-link disabled">Previous</a>
    </li>
    <li class="page-item">
      <a class="page-link disabled" href="#">1</a>
    </li>
    <li class="page-item" aria-current="page">
      <a class="page-link disabled" href="#">2</a>
    </li>
    <li class="page-item">
      <a class="page-link disabled" href="#">3</a>
    </li>
    <li class="page-item">
      <a class="page-link disabled" href="#">Next</a>
    </li>
  </ul>
</nav>
{{< /example >}}

scss/_variables.scss Show resolved Hide resolved
scss/_variables.scss Show resolved Hide resolved
scss/_variables.scss Show resolved Hide resolved
site/content/docs/5.1/components/alerts.md Show resolved Hide resolved
site/content/docs/5.1/components/dropdowns.md Show resolved Hide resolved
@julien-deramond julien-deramond merged commit e8b9f20 into main Mar 31, 2022
@julien-deramond julien-deramond deleted the chore/merge-main@7f35bc5 branch March 31, 2022 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge Sync with Bootstrap v5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants