Apply visible focus design guidelines in all components #1437
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
louismaximepiton
force-pushed
the
main-mlh-apply-new-visible-focus-all-components
branch
2 times, most recently
from
2641c61 to
5e5b7d3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
We should move to :focus-visible and drop the focus-visible dependency. Implies to check that it's well implemented in all forms + check that we don't have any reference to it in scss files.
Should be rebased on main with new focus rule from Bootstrap!
Should we introduce Sass variable for each component redefining their own focus style ? any thoughts on this particular question @julien-deramond ?
On other files:
- Should refactor in
search.scss - Should refactor in
reboot.scssthe[type="search"](Someone with Mac @isabellechanclou) - In
button-group.scss, l.137 is useful ? - Do we need rules on offset and focus inside
anchor.scss - In
buttons.scss, check if we could revert chore/merge-main@2504b89 #1513 (comment) once rebased -> Wait&see what Bs will do since our version seems to be easier and works well. 💡 Maybe change Bs management of.btn-chekck + .btn - In
variables.scss:-
$input-btn-focus-widthshould take$outline-offsetvalue -
$input-btn-focus-box-shadowshould be almost the same as$btn-focus-box-shadow-> Fine as-is as we don't want input to have a focus-box.
-
- In
colored-links.scss, we should remove:focusrule - In
mixin/list-groupshould remove:focus - Is there anything to change on
.visually-hidden? - In all files, see if we can remove easily the
:not(focus)rules (in button-group) -> May be handled in Fixes for dropdown #1164 and other PRs - Check every focus rules inside
assets/scss/**.scss(boosted, clipboard, navbar) - [OUT OF PR] Check assets/boosted.scss to see if the rules with id are still correct.
- [OUT OF PR] Checks&radios should have a different id.
- [OUT OF PR] Carousel indicators are broken if we hover for too long
This comment was marked as outdated.
This comment was marked as outdated.
MewenLeHo
force-pushed
the
main-mlh-apply-new-visible-focus-all-components
branch
from
0f27f2e to
a5c30bb
Compare
There was a problem hiding this comment.
Package-lock.json shouldn't be commited maybe related to ff9ed90
MewenLeHo
force-pushed
the
main-mlh-apply-new-visible-focus-all-components
branch
from
94db821 to
fcff75c
Compare
MewenLeHo
force-pushed
the
main-mlh-apply-new-visible-focus-all-components
branch
from
09a5970 to
a055e1b
Compare
There was a problem hiding this comment.
As we said: should we add a focus-box on button-group only (and not on every button) ?
We may handle the deploy preview, any thoughts @julien-deramond ? Sorry for the ping, Seems to be still imported in generate-sri file.
We need to clean-up the doc to be sure that there isn't any more a reference to this dependency.
This comment was marked as outdated.
This comment was marked as outdated.
|
Kudos, SonarCloud Quality Gate passed!
|
julien-deramond
deleted the
main-mlh-apply-new-visible-focus-all-components
branch
Fixes #398.
Fixes #659.
Fixes #1035.
Fixes #1189.
Fixes #1189.
Commit that could be removed: 449d2c9 (here to reduce bundle size ~.05KB once packed, but may reduce comprehension ?)