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.

Sign up for GitHub

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

Navbar scroll: unintended scroll while tabing inside navbar #1274

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Navbar scroll: unintended scroll while tabing inside navbar #1274

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ffe0d88
First attempt
louismaximepiton Jul 4, 2022
71c8d85
Merge branch 'main' into main-lmp-navbar-scroll-fix
louismaximepiton Nov 9, 2022
97ae625
Handle the differents sizes of header
louismaximepiton Nov 9, 2022
81ad1a8
Merge branch 'main' into main-lmp-navbar-scroll-fix
julien-deramond Dec 26, 2022
2629dc9
Merge branch 'main' into main-lmp-navbar-scroll-fix
louismaximepiton Jan 18, 2023
26b3210
Actualizing the CSS and js to be smaller and leave less
louismaximepiton Jan 18, 2023
d0d5675
fix tests
louismaximepiton Jan 18, 2023
55dca2c
Merge branch 'main' into main-lmp-navbar-scroll-fix
louismaximepiton Feb 14, 2023
c50284a
Split this PR in two.
louismaximepiton Feb 14, 2023
fb4c0e3
Re introduce things that shouldn't be removed.
louismaximepiton Feb 14, 2023
f01be59
Merge branch 'main' into main-lmp-navbar-scroll-fix
louismaximepiton Mar 21, 2023
fb4c064
Enhance the a11y message
louismaximepiton Mar 21, 2023
74ba5ff
Merge branch 'main' into main-lmp-navbar-scroll-fix
louismaximepiton Apr 25, 2023
4d6b77f
fix(review)
louismaximepiton May 30, 2023
334828d
Merge branch 'main' into main-lmp-navbar-scroll-fix
julien-deramond Jul 27, 2023
fc61c2f
Fix lint
julien-deramond Jul 27, 2023
cbe8292
Values adjustements
julien-deramond Jul 27, 2023
1d48c99
Update migration guide and envariable some values to avoid changing them
louismaximepiton Jul 27, 2023
7c1607c
Change default value of $enable-fixed-header in the docs
julien-deramond Jul 28, 2023
770be41
Merge remote-tracking branch 'origin/main' into main-lmp-navbar-scrol…
louismaximepiton Nov 30, 2023
a6bdb91
Introduce `scroll-margin` instead of `scroll-padding`
louismaximepiton Nov 30, 2023
3fe8b9b
.
louismaximepiton Nov 30, 2023
bbe0bd3
Merge branch 'main' into main-lmp-navbar-scroll-fix
louismaximepiton Jan 4, 2024
de2f723
.
louismaximepiton Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions scss/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ $enable-validation-icons: true !default;
$enable-negative-margins: false !default;
$enable-deprecation-messages: false !default;
$enable-important-utilities: true !default;
$enable-fixed-header: true !default; // Boosted mod: used to apply scroll-padding-top
$enable-fixed-header: false !default; // Boosted mod: used to apply scroll-padding-top
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that the default value of $enable-fixed-header should be false?

Taking https://deploy-preview-1274--boosted.netlify.app/docs/5.3/examples/navbar-sticky/ as an example which doesn't work as expected. Maybe https://github.com/Orange-OpenSource/Orange-Boosted-Bootstrap/blob/main/site/content/docs/5.3/examples/navbar-sticky/navbar-sticky.css needs to be modified in this PR as well.


$enable-dark-mode: true !default;
$color-mode-type: data !default; // `data` or `media-query`
Expand Down Expand Up @@ -2225,7 +2225,7 @@ $pre-line-height: 1.25 !default;
//

//// Scroll margin
$scroll-offset-top: $spacer * 6 !default; // Matching .navbar computed height
$scroll-offset-top: $spacer * 5 !default; // Matching .navbar computed height

//// Back to top
// scss-docs-start back-to-top
Expand Down
8 changes: 0 additions & 8 deletions site/assets/scss/_boosted.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
// stylelint-disable selector-max-id
:root {
scroll-padding-top: $offset-top / 2;

@include media-breakpoint-up(lg) {
scroll-padding-top: $offset-top;
}
}

html,
body {
min-height: 100vh;
Expand Down
10 changes: 9 additions & 1 deletion site/assets/scss/_scrolling.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,15 @@ main {
h3,
h4,
[tabindex="0"] {
scroll-margin-top: 80px;
scroll-margin-top: 7.5rem;
scroll-margin-bottom: 100px;

@include media-breakpoint-up(md) {
scroll-margin-top: 11rem;
}

@include media-breakpoint-up(lg) {
scroll-margin-top: 8rem;
}
}
}
4 changes: 4 additions & 0 deletions site/content/docs/5.3/getting-started/accessibility.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ Each component then specifies its own visible focus style when needed, for examp

#### Under a fixed header

{{< callout warning >}}
isabellechanclou marked this conversation as resolved.
Show resolved Hide resolved
Know that using this feature might lead to unexpected scroll while tabbing inside the header. Prefer `scroll-margin-*` on focusable elements.
{{< /callout >}}

When using a fixed (or sticky) header, tabbing backward often hides focused element under the header. Boosted sets `scroll-padding-top` property for such case. This feature is configurable in two ways:

1. `$scroll-offset-top` variable defines the offset,
Expand Down