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

Dataset#details : ajout modal pour conversion auto NeTEx #4452

Merged
merged 5 commits into from
Feb 25, 2025
Merged

Dataset#details : ajout modal pour conversion auto NeTEx #4452

merged 5 commits into from
Feb 25, 2025

Conversation

AntoineAugusti
Copy link
Member

Ajoute une modal sur la page de JDD lorsqu'une conversion automatique NeTEx est proposée en précisant lors d'une tentative de téléchargement que cette fonctionnalité ne sera plus disponible à partir de juin 2025.

Screen.Recording.2025-02-19.at.10.24.41.mov

@AntoineAugusti AntoineAugusti force-pushed the dataset_details_netex_conversion_modal branch from 9b1ea0d to 09aa127 Compare February 19, 2025 09:41
@AntoineAugusti AntoineAugusti mentioned this pull request Feb 19, 2025
Open
5 tasks
@AntoineAugusti AntoineAugusti force-pushed the dataset_details_netex_conversion_modal branch from 09aa127 to a7ef77e Compare February 19, 2025 09:45
@AntoineAugusti AntoineAugusti marked this pull request as ready for review February 19, 2025 09:48
@AntoineAugusti AntoineAugusti requested a review from a team as a code owner February 19, 2025 09:48
@AntoineAugusti AntoineAugusti force-pushed the dataset_details_netex_conversion_modal branch from a7ef77e to c0f626a Compare February 19, 2025 11:05
@AntoineAugusti AntoineAugusti force-pushed the dataset_details_netex_conversion_modal branch from c0f626a to 51e2228 Compare February 19, 2025 11:08
@ptitfred ptitfred self-assigned this Feb 19, 2025
@ptitfred
Copy link
Contributor

La modale des conversions NeTEx est utilisée à 2 endroits différents ; ça se comporte bien dans les deux cas ?

ptitfred
ptitfred reviewed Feb 20, 2025
View reviewed changes
apps/transport/client/stylesheets/components/_dataset-details.scss

.modal .button__group {
a.button:hover {
background-color: var(--theme-secondary-darken) !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ces deux !important sont symptomatiques d'un problème dans notre css :/

Pas sûr qu'on veuille s'y pencher ici cela dit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oui, il y a un !important .resource a:hover qui fait du bazard ici…

ptitfred
ptitfred reviewed Feb 20, 2025
View reviewed changes
Copy link
Contributor

@ptitfred ptitfred left a comment

Choose a reason for hiding this comment

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

Quelques suggestions mais rien de dramatique.

apps/transport/client/stylesheets/components/_dataset-details.scss Outdated
@@ -635,3 +635,12 @@ hr {
}
}
}

.modal .button__group {
Copy link
Contributor

Choose a reason for hiding this comment

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

On devrait sans doute rendre ce sélecteur plus spécifique, notamment pour éviter que ce comportement n'affecte d'autres pages.

L'élément de top-level du template de la modale n'inclut pas de bonne classe CSS pour la rendre spécifique. Peut-être quelque chose comme .netex-conversion-modal ?

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est fait !

AntoineAugusti
AntoineAugusti commented Feb 21, 2025
View reviewed changes
apps/transport/client/stylesheets/components/_dataset-details.scss

.modal .button__group {
a.button:hover {
background-color: var(--theme-secondary-darken) !important;
Copy link
Member Author

Choose a reason for hiding this comment

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

Oui, il y a un !important .resource a:hover qui fait du bazard ici…

@AntoineAugusti @ptitfred
Apply suggestions from code review
d63e7c0 
Co-authored-by: Frédéric Menou <[email protected]>
@AntoineAugusti
Copy link
Member Author

La modale des conversions NeTEx est utilisée à 2 endroits différents ; ça se comporte bien dans les deux cas ?

Oui, ça fonctionne comme attendu sur dataset#details et resource#details, tu peux vérifier en local si tu souhaites !

@AntoineAugusti AntoineAugusti force-pushed the dataset_details_netex_conversion_modal branch from 6bbdbec to 9dddff2 Compare February 24, 2025 08:34
@AntoineAugusti AntoineAugusti added this pull request to the merge queue Feb 25, 2025
Merged via the queue into master with commit 4b649f8 Feb 25, 2025
4 checks passed
@AntoineAugusti AntoineAugusti deleted the dataset_details_netex_conversion_modal branch February 25, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ptitfred ptitfred ptitfred approved these changes

Assignees

@ptitfred ptitfred

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AntoineAugusti @ptitfred