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

Jump to bottom

Update dsfr #249

Merged
merged 30 commits into from
Jun 14, 2023
Merged

Update dsfr #249

merged 30 commits into from
Jun 14, 2023

Conversation

nicolaskempf57
Copy link
Contributor

@nicolaskempf57 nicolaskempf57 commented May 15, 2023 •
edited
Loading

@nicolaskempf57
chore: ⬆️ update dsfr
bda727f
@nicolaskempf57
fix: 💄 fix DSFR removed style
10dd5fc
@nicolaskempf57
refactor: 🔥 remove code added to dsfr
8d71b1b
@nicolaskempf57
refactor: 🔥 remove code from fixed dsfr bug
453f4f5
@nicolaskempf57
fix: 🐛 fix dsfr notice style not imported
0749778
@nicolaskempf57
fix: 🐛 fix dsfr tab without js
5e3610e
@nicolaskempf57
fix: 🐛 fix list sanitize
5bbdbb4
@nicolaskempf57
fix(dsfr): 🐛 fix not enlarged link
d1b4219
@nicolaskempf57
fix(dsfr): 🐛 fix selected tag color
28e4889
@nicolaskempf57
fix(dsfr): 🐛 fix pressed check color
dad86fc
@nicolaskempf57
fix(dsfr): 🐛 fix sanitize li
88a467b
@nicolaskempf57
feat(dsfr): ✨ add button to change lang
1dc88f3
@nicolaskempf57
feat(dsfr): ✨ use dsfr translate component to select language
aa70cef
@nicolaskempf57
feat(dsfr): 💬 update footer title
21f2941
@nicolaskempf57
feat(dsfr): ✨ add email hint
2fc23a2
@nicolaskempf57
Merge branch 'master' into feat/update-dsfr-to-1.7
89e90a5
@nicolaskempf57
refactor(dsfr): ♻️ use aria-labelledby instead of aria-label
42805ff
@nicolaskempf57
fix(dsfr): 💄 fix border
a714b7c
@nicolaskempf57
refactor(dsfr): 🔥 remove unecessary code after dsfr update
c706b3b
@nicolaskempf57
refactor: ♻️ use h3 for all cards
ded86b3
@nicolaskempf57
docs: 📝 fix function doc
83f9cd3
@nicolaskempf57
style: 🎨 add whitespace
101c846
@nicolaskempf57
fix: 🐛 fix bug with error page
81a4a12
@nicolaskempf57
style: 🎨 whitespace for qa
45cd7b8
@nicolaskempf57
style: 🎨 whitespace
c2f8b28
@nicolaskempf57
style: 🎨 whitespace qa
b3b68da
@nicolaskempf57 nicolaskempf57 requested a review from maudetes June 5, 2023 12:43
maudetes
maudetes reviewed Jun 6, 2023
View reviewed changes
Copy link
Contributor

@maudetes maudetes left a comment

Choose a reason for hiding this comment

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

Nice, I have only minor comments!

  • don't forget the changelog
  • some links are now underlined (license, orga on dataset card), has it been made on purpose?

udata_front/theme/gouvfr/assets/less/override.less Outdated
@@ -52,3 +52,8 @@ a:not([href]) {
Menlo, Monaco, Consolas,
monospace;
}

:where(nav li):before {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is that for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our Sanitize.css adds a • to the left of <li>. This rules was never triggered before but it adds really wide padding to our list of lang. There are PR in sanitize to fix this (csstools/sanitize.css#227, csstools/sanitize.css#234) but the project seems inactive.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember what we've said before on

  • , but we don't have any list showing right now or with this PR right?

    •

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    This is only for <li> in <nav> and it didn't have this side effect on our header. But, we now have a list in the new translate component.

    Copy link
    Contributor

    @maudetes maudetes Jun 13, 2023 •
    edited
    Loading

    Choose a reason for hiding this comment

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

    Oh my, I see. But <li> are not showing correctly, example on https://www.data.gouv.fr/fr/datasets/table-de-passage-geographie-2022/, where Dans la cellule D2,, etc. is a <li> showing as a raw string.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    We must add number back to <ol>in markdown. We already did it for <ul>.

    @nicolaskempf57
    Copy link
    Contributor Author

    • some links are now underlined (license, orga on dataset card), has it been made on purpose?

    These wasn't possible before due to how DSFR displayed underlines. But they changed it so I added them.

    Orga links fail RGAA criterion 3.1 without the underline.

    For license, it's valid without underline but you can't know that it's a link without it.

    @nicolaskempf57 nicolaskempf57 requested a review from maudetes June 13, 2023 07:30
    maudetes
    maudetes approved these changes Jun 13, 2023
    View reviewed changes
    Copy link
    Contributor

    @maudetes maudetes left a comment

    Choose a reason for hiding this comment

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

    I think I'm good with this PR 👏

    We can deal with <ol> support now or later :)

    @nicolaskempf57 nicolaskempf57 merged commit 7b75884 into datagouv:master Jun 14, 2023
    @nicolaskempf57 nicolaskempf57 deleted the feat/update-dsfr-to-1.7 branch June 14, 2023 12:11
    @nicolaskempf57 nicolaskempf57 mentioned this pull request Jan 15, 2025
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @maudetes maudetes maudetes approved these changes

    Assignees
    No one assigned
    Labels
    None yet
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    Mise à jour du DSFR v1.6 & 1.7
    2 participants
    @nicolaskempf57 @maudetes