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@795167d #1247

Merged
merged 71 commits into from
May 13, 2022

Conversation

julien-deramond
Copy link
Contributor

@julien-deramond julien-deramond commented May 11, 2022

@julien-deramond julien-deramond added v5 merge Sync with Bootstrap skip:ci labels May 11, 2022
@louismaximepiton louismaximepiton mentioned this pull request May 12, 2022
8 tasks
@@ -27,7 +27,7 @@ Badges can be used as part of links or buttons to provide a counter.

{{< example >}}
<button type="button" class="btn btn-primary">
Notifications <span class="badge bg-secondary">4</span>
Notifications <span class="badge text-bg-secondary">4</span>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to change the positionned example or not ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is something I don't really understand in Bootstrap's modification. They use here .text-bg-secondary but the default value is already white. Two options:

  • Remove the default color of badge with the CSS var and use everywhere in all examples text-bg-*
  • Don't use here .text-bg-secondary because it doesn't bring anything
    Thoughts?

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.

do we need to explain that scrollspy support now accordion like elements ?
Reviewed 76/84 files

site/content/docs/5.1/components/navs-tabs.md Show resolved Hide resolved

<div class="offcanvas-lg offcanvas-end" tabindex="-1" id="offcanvasResponsive" aria-labelledby="offcanvasResponsiveLabel">
<div class="offcanvas-header">
<h5 class="offcanvas-title" id="offcanvasResponsiveLabel">Responsive offcanvas</h5>
<button type="button" class="btn-close" data-bs-dismiss="offcanvas" aria-label="Close"></button>
<button type="button" class="btn-close" data-bs-dismiss="offcanvas" data-bs-target="#offcanvasResponsive"><span class="visually-hidden">Close</span></button>
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need explicit target on this one and not on the others ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without it:

Uncaught TypeError: this._config is undefined
    _initializeBackDrop offcanvas.js:179
    Offcanvas offcanvas.js:71
    getOrCreateInstance base-component.js:65

I haven't looked at it in details but I find it weird. This is maybe a bug. Is it mentioned in the PR?

site/content/docs/5.1/components/toasts.md Outdated Show resolved Hide resolved
site/content/docs/5.1/components/toasts.md Show resolved Hide resolved
site/layouts/_default/docs.html Show resolved Hide resolved
site/content/docs/5.1/components/scrollspy.md Show resolved Hide resolved

Scrollspy also works with nested `.nav`s. If a nested `.nav` is `.active`, its parents will also be `.active`. Scroll the area next to the navbar and watch the active class change.

<div class="bd-example">
<div class="row">
<div class="col-4 col-lg-3">
<nav id="navbar-example3" class="navbar flex-column align-items-stretch">
Copy link
Member

Choose a reason for hiding this comment

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

This is not backported just below l.178

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bootstrap side

site/content/docs/5.1/components/scrollspy.md Show resolved Hide resolved
site/content/docs/5.1/components/scrollspy.md Show resolved Hide resolved
site/assets/scss/_component-examples.scss Show resolved Hide resolved
site/assets/scss/_component-examples.scss Show resolved Hide resolved
site/data/sidebar.yml Show resolved Hide resolved
site/data/sidebar.yml Show resolved Hide resolved
site/assets/scss/_component-examples.scss Show resolved Hide resolved
@julien-deramond julien-deramond marked this pull request as ready for review May 13, 2022 12:17
@julien-deramond julien-deramond merged commit 0d4a04b into main-jd-refactor-design-doc May 13, 2022
@julien-deramond julien-deramond deleted the chore/merge-main@795167d branch May 13, 2022 12:24
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