Redesign de la fiche détaillée #968
Conversation
fabienheureux
force-pushed
the
actor-detail-redesign
branch
from
479519e
to
cd0b426
Compare
fabienheureux
force-pushed
the
actor-detail-redesign
branch
from
cd0b426
to
e048df6
Compare
fabienheureux
force-pushed
the
main
branch
20 times, most recently
from
d90d030
to
2eae9d3
Compare
| @@ -22,5 +19,6 @@ test("Recherche et modification d'une recherche", async ({ page }) => { | |||
|
|
|||
| await expect (page.locator("[data-search-solution-form-target=headerAddressPanel]")).toBeInViewport() | |||
| await page.locator("button[data-testid=modifier-recherche]").click() | |||
| await expect (page.locator("[data-search-solution-form-target=headerAddressPanel]")).toBeHidden() | |||
| // TODO : revert | |||
There was a problem hiding this comment.
On adapt le test ou on le supprime ?
| class TestAdessesViewGetActionList: | ||
| @pytest.mark.django_db | ||
| class TestAdresseViewMixins: | ||
| def test_iframe_mixin_is_carte(self): |
There was a problem hiding this comment.
ajouter un test test_iframe_mixin_is_not_… ?
Co-authored-by: Nicolas Oudard <[email protected]>
Co-authored-by: Nicolas Oudard <[email protected]>
| location = acteur.location | ||
|
|
||
| if not (longitude and latitude and location and not acteur.is_digital): | ||
| return "" |
There was a problem hiding this comment.
Le pattern python quand on veut signaler un manque c'est None: raison particulière pour du ""?
There was a problem hiding this comment.
OK je vois qu'on est dans du jinja donc je comprend le "", mais du coup voir ma remarque ci-dessous sur la séparation modelling vs. rendering.
| * 111320 | ||
| ) | ||
| if distance_meters >= 1000: | ||
| return f"({round(distance_meters / 1000, 1)} km)".replace(".", ",") |
There was a problem hiding this comment.
Je m'attendrais à séparer:
- la logique de modelling: une fonction qui retourne une distance en numérique, qu'on peut utiliser dans plusieurs endroits du code
- le rendering: un template qui prend la valeur de distance et décide de la logique l'affichage pour les êtres humains
There was a problem hiding this comment.
Nico avait mentionné une refacto avec des règles de nommage et séparation des concerns (ex: _labels avec un _ vs bonus sans): as-tu documenté/formalisé sur Notion? Si non je pense qu'il faudrait pour que je me mette à niveau 🙏
| @@ -10,47 +10,6 @@ const DEFAULT_LOCATION: L.LatLngTuple = [46.227638, 2.213749] | |||
| const DEFAULT_ZOOM: number = 5 | |||
| const DEFAULT_MAX_ZOOM: number = 19 | |||
|
|
|||
| // TODO : handle directly from DSFR module | |||
There was a problem hiding this comment.
@kolok penses-tu qu'il faille conserver quelque part ce mapping ?
| # Even if this variable is mainly used when navigating on the Carte | ||
| # or Formulaire views, it is used in templates as JSON and parsed to | ||
| # be passed to Posthog analytics. | ||
| # It is then required to be added on every view by default, because | ||
| # the JSON parsing might raise an exception if the variable is undefined. | ||
| "is_embedded": "carte" in request.GET or "iframe" in request.GET, | ||
| } |
There was a problem hiding this comment.
@maxcorbeau @kolok est-ce clair ?
Description succincte du problème résolu
Carte notion
TODO
Description plus détaillée de l'intention, l'approche ou de l'implémentation (ce qui n’est pas visible directement en lisant le code)
Type de changement :
Auto-review
Les trucs à faire avant de demander une review :
.env.templateComment tester
En local / staging :