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

[GEN-129] UX/UI: page nouveautés #4562

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Conversation

calummackervoy
Copy link
Contributor

@calummackervoy calummackervoy commented Aug 15, 2024

🤔 Pourquoi ?

Aujourd’hui, le seul espace qui permet de connaitre les modifications et nouveautés se trouve dans le journal de modifications, qui n’est pas du tout digeste pour un utilisateur.
Offrons à nos utilisateurs un espace qui regroupe chaque mois les fonctionnalités qui ont été ajoutées / modifiées

🍰 Comment ?

  • Nouvelle page /news/
  • Liens vers les nouveautés (header, modale) réderigent à cette page
  • Stockage des images sur une nouvelle collection publique dans le config bucket
  • Widget pour l'image dans le site admin
  • Nouvelle dépendance: Pillow

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?

💻 Captures d'écran

Screenshot 2024-08-15 at 11 29 15

Modification dans le site admin :

Screenshot 2024-08-15 at 11 27 22

@calummackervoy calummackervoy added the ajouté Ajouté dans le changelog. label Aug 15, 2024
@calummackervoy calummackervoy self-assigned this Aug 15, 2024
Copy link

socket-security bot commented Aug 15, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
pypi/[email protected] environment, eval, filesystem, shell 0 17.1 MB aclark, hugovk, radarhere, ...1 more

🚮 Removed packages: pypi/[email protected])

View full report↗︎

@calummackervoy calummackervoy force-pushed the calum/page-nouveautes branch 2 times, most recently from f70657f to e3938ab Compare August 15, 2024 14:21
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Ça marche plutôt bien chez moi, une bonne première version. 👏

Il faudra passer un coup de --snapshot-update:
Deleted TestNewsRender.test_campaign_rendered_dashboard (tests/www/news/snapshots/test_views.ambr)

config/settings/base.py Outdated Show resolved Hide resolved
itou/communications/admin.py Show resolved Hide resolved
itou/communications/forms.py Outdated Show resolved Hide resolved
itou/communications/forms.py Outdated Show resolved Hide resolved
itou/communications/forms.py Outdated Show resolved Hide resolved
tests/communications/test_campaigns.py Outdated Show resolved Hide resolved
tests/communications/test_models.py Outdated Show resolved Hide resolved
tests/www/news/test_views.py Outdated Show resolved Hide resolved
tests/www/news/test_views.py Outdated Show resolved Hide resolved
tests/www/news/test_views.py Outdated Show resolved Hide resolved
@hellodeloo
Copy link
Contributor

@calummackervoy Y a-t-il une carte notion et un figma associé à cette PR ? Si oui, je veux bien les urls. Merci

@calummackervoy
Copy link
Contributor Author

Copy link
Contributor

@hellodeloo hellodeloo left a comment

Choose a reason for hiding this comment

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

@calummackervoy J'ai pushé un petit commit d'adaptation ui.
Aussi, je pense qu'il manque le lien "Retour" dans le {% block title_prevstep %}

@hellodeloo hellodeloo force-pushed the calum/page-nouveautes branch 2 times, most recently from 6f8e21b to 7ae3a66 Compare August 20, 2024 07:58
@calummackervoy calummackervoy force-pushed the calum/page-nouveautes branch 3 times, most recently from 9d6e162 to 9731a93 Compare August 27, 2024 13:46
@calummackervoy calummackervoy added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Aug 27, 2024
@calummackervoy calummackervoy added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC and removed 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC labels Aug 27, 2024
Copy link

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Ça devient très agréable à utiliser 👍

itou/communications/forms.py Outdated Show resolved Hide resolved
itou/www/news/views.py Outdated Show resolved Hide resolved
itou/templates/news/news.html Outdated Show resolved Hide resolved
itou/templates/news/news.html Outdated Show resolved Hide resolved
itou/templates/news/news.html Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
itou/templates/news/news.html Outdated Show resolved Hide resolved
assert len(response.context["news_page"]) == 1
assert response.context["news_page"][0] == campaign

@pytest.mark.ignore_unknown_variable_template_error("boost_target", "boost_indicator")
Copy link
Contributor

Choose a reason for hiding this comment

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

N’ignorons pas les erreurs dans du nouveau code. La mark est présente pour avoir une baseline sur le code écrit avec qu’on ne soit capable de vérifier que toutes les variables d’un template étaient définies.

Suggested change
@pytest.mark.ignore_unknown_variable_template_error("boost_target", "boost_indicator")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK ! Merci 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je pense que j'ai besoin de cela ; sans le j'ai un erreur Undefined template variable 'boost' mais on n'utilise que {% if boost %} donc c'est valide :S

Je l'ai copié de la classe TestApprovalsListView et j'imagine qu'il était conçu en réponse à ce problème

tests/www/news/test_views.py Outdated Show resolved Hide resolved
tests/www/news/test_views.py Outdated Show resolved Hide resolved
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Ça m’a l’air très pratique à utiliser !
Quelques petites remarques à corriger et je pense qu’on sera bons pour le rendre disponible 🎯

Copy link
Contributor

Choose a reason for hiding this comment

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

Si on souhaite pousser le vice (et améliorer la cohérence avec les noms des modèles), on pourrait aussi renommer ce template et le module de www. Comme tu préfères.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'étais entre deux esprits sur le nom annonce plus générique et news qui comporte mieux nos besoins courrants. Enfin j'ai réorganisé ces noms pour être plus logique. J'ai donc bougé la page à /announcements/news/. Un jour ça pourrait arriver que l'appli announcements héberge les autres utilisations et l'idée se vindiquera 🤞

itou/communications/forms.py Outdated Show resolved Hide resolved
itou/static/css/itou.css Outdated Show resolved Hide resolved
itou/templates/news/news.html Outdated Show resolved Hide resolved
itou/templates/news/news.html Outdated Show resolved Hide resolved
itou/templates/news/news.html Outdated Show resolved Hide resolved
tests/www/news/test_views.py Outdated Show resolved Hide resolved
@francoisfreitag
Copy link
Contributor

La CI est corrigée avec #4626

@francoisfreitag
Copy link
Contributor

Precise new headings and subheadings for improved HTML semantic

J’ai ajouté une mini modif du markup pour essayer d’améliorer la sémantique de la page, <div> et <p> n’étant pas idéaux pour indiquer la hiérarchie du contenu.

@francoisfreitag
Copy link
Contributor

francoisfreitag commented Sep 4, 2024

J’ai ajouté une mini modif du markup pour essayer d’améliorer la sémantique de la page, <div> et <p> n’étant pas idéaux pour indiquer la hiérarchie du contenu.

Que je viens de revert, car on ne peut pas mettre de heading dans un bouton. Laissons tel quel.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#technical_summary

itou/communications/forms.py Outdated Show resolved Hide resolved
itou/templates/announcements/news.html Outdated Show resolved Hide resolved
@francoisfreitag
Copy link
Contributor

Ça m’a l’air top 💯 !

@calummackervoy
Copy link
Contributor Author

calummackervoy commented Sep 4, 2024

@francoisfreitag j'attends que ton PR le #4659 et on peut le merger 🎉 la recette jetable a été validée

@calummackervoy calummackervoy force-pushed the calum/page-nouveautes branch 3 times, most recently from dd63df2 to 69c7f71 Compare September 9, 2024 06:55
feat(communications): render news modal on first device visit

feat(communications/admin): AnnouncementCampaign admin and validation

fix(AnnouncementCampaignAdminForm): modifying date range

feat(communications): render news modal on first device visit

feat(communications): implemented a cache for AnnouncementCampaign

greatly restricts database usage

refactor(CommunicationsConfig): django signals safe to import at module level

refactor(communications): model changes requested in feedback

refactor(communications): implemented requested cache changes

fix: make quality

fix(static): delete unused image file

refactor(communications): cache changes, added live field

feat(communications): render news modal on first device visit

feat(communications/admin): AnnouncementCampaign admin and validation

fix(AnnouncementCampaignAdminForm): modifying date range

feat(communications): render news modal on first device visit

feat(communications): implemented a cache for AnnouncementCampaign

greatly restricts database usage

refactor(communications): implemented requested cache changes

feat(communications): news item model fields, admin and form

WIP: page nouveautés

feat(news.html): styling for news items

fix: merge error

requirements: merge errors

fix: test and code quality

feat(news): limit candidate news to relevant news only

feat(news.html): collapse icon on monthly news summaries

fix: rebase errors

feat: links to news page

fix: code quality

fix(tests): reset_model_sequence for snapshot

fix: some ui adjusments

refactor(communications/admin.py): change import style

refactor(communications/forms.py): relying more on Python core

feat(communications/models.py): alt_text field, other model changes

refactor(configure_bucket): more concise version of configuration

WIP: diverse requested changes

feat(news): pagination

refactor: enhance testing

fix(settings): more specific addition to CSP_IMG_SRC policy

fix: tests

fix: tests

fix: content policy

fix: conflict

fix: link to news in header content

fix: quality

fix(news): standardise image height

fix(AnnouncementItemForm): restrict user_kind_tags

following the spec

refactor: requested changes

fix: quality error

fix: test failure on pipeline

refactor: requested changes

refactor: consistent naming convention on news

fix: rebase mistake

minor fixes

fix: quality

fix: CSS classes, comments

Precise new headings and subheadings for improved HTML semantic

Revert "Precise new headings and subheadings for improved HTML semantic"

This reverts commit e9b09f7.

Actually, headings aren’t allowed under button. Let’s leave it at that.

minor adjustments

fix: import
@calummackervoy calummackervoy added this pull request to the merge queue Sep 9, 2024
Merged via the queue into master with commit 1ad3fa1 Sep 9, 2024
10 of 11 checks passed
@calummackervoy calummackervoy deleted the calum/page-nouveautes branch September 9, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC ajouté Ajouté dans le changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants