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

[Tableau de bord] Ajout du bloc AMP #1722

Merged
merged 6 commits into from
Oct 2, 2024

Conversation

maximeperraultdev
Copy link
Collaborator

Related Pull Requests & Issues


  • Tests E2E (Cypress)

@@ -160,6 +148,8 @@ export function DashboardForm() {
const Container = styled(SideWindowContent)`
display: flex;
flex-direction: row;
gap: 48px;
padding: 24px 0 0 25px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

pourquoi ce padding? normalement SideWindowContent a déjà la bon padding (même style pour les signalements et les missions)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

C'est le padding de la maquette.

@@ -170,5 +160,5 @@ const Column = styled.div`
height: calc(100vh- 48px - 40px); // 48px = navbar height, 40px = padding
overflow-y: auto;
overflow-x: visible;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourquoi avoir enlever le padding? Il y a maintenant pose un souci d'affichage :
Sans le padding :
Capture d’écran 2024-09-27 à 16 01 28

Avec le padding:

Capture d’écran 2024-09-27 à 16 01 45

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

étrange que le padding fasse apparaitre la bordure. Merci de l'avoir vu

Copy link
Collaborator

@claire2212 claire2212 left a comment

Choose a reason for hiding this comment

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

Pout tous les changements dans le dossier RegulatoryAreas il y en a une partie que j'ai fait dans ma PR. Pour le reste (notamment dans le fichier Layer, j'ai fait autrement)

@@ -71,3 +71,7 @@ const AccordionContent = styled.div<{ $isExpanded: boolean }>`
max-height: ${({ $isExpanded }) => ($isExpanded ? '100vh' : '0px')};
transition: ${({ $isExpanded }) => ($isExpanded ? '0.5s max-height ease-in' : '0.3s max-height ease-out')};
`

export const AccordionWrapper = styled.div`
padding: 3px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

pourquoi ne pas ajouter ce padding sur le composant Column ? ça éviterait de l'ajouter à chaque fois


const isSubPanelOpened = !!(openPanel?.subPanel?.id && openPanel.subPanel.type === Dashboard.Block.AMP)

const onClick = (event, id: number | undefined) => {
Copy link
Collaborator

@claire2212 claire2212 Oct 1, 2024

Choose a reason for hiding this comment

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

possible de mettre un nom de fonction un peu moins générique ?

  • je viens de me rendre compte qu'il faudrait masquer le panel quand il est affiché et qu'on re-clique sur l'icône (idem côté Zones reg) à faire plus tard peut-être

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je me posais la question je croyais que c'était voulu ^^

)
const openPanel = useAppSelector(state =>
activeDashboardId
? getOpenedPanel(state.dashboard, { id: activeDashboardId, type: Dashboard.Block.VIGILANCE_AREAS })
Copy link
Collaborator

@claire2212 claire2212 Oct 1, 2024

Choose a reason for hiding this comment

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

je pense qu'il n'y a pas besoin de passer activeDashboardId puisqu'il est déjà dans le state. Tu peux le récupérer directement dans le selector. Ca éviterait la condition en plus activeDashboardId ? xxx : xxxx

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, bien vu

@@ -66,87 +73,64 @@ export function RegulatoryAreas({ regulatoryAreaIds }: { regulatoryAreaIds: Arra

return (
<>
{regulatoryAreaId && <StyledRegulatoryAreasPanel layerId={regulatoryAreaId} onClose={closeRegulatoryAreapanel} />}
{openPanel?.subPanel && isSubPanelOpened && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

le openPanel?.subPanel n'est pas redondant comment tu vérifies déjà openPanel?.subPanel?.id pour définir isSubPanelOpened ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je suis d'accord mais j'ai quand meme une erreur de typage...

<PanelSubPart>
<PanelInlineItemLabel>Réglementations en lien</PanelInlineItemLabel>
{regulatoryAreas &&
regulatoryAreas.length > 0 &&
Copy link
Collaborator

@claire2212 claire2212 Oct 1, 2024

Choose a reason for hiding this comment

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

pourquoi l'enlever? d'expérience j'ai déjà eu des soucis de vérif juste sur array qui ne passait pas alors que array.length > 0 passait

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je ne vois pas pourquoi ? ici on vérifie que le tableau n'est pas undefined s'il n'est pas undefined alors on boucle sur la map qui ne fera rien si le tableau est vide. la condition sur le composant parent est nécessaire pour ne pas afficher le header s'il n'y a pas d'éléments par contre.


<ButtonsContainer>
<StyledButton
accent={Accent.TERTIARY}
color={regulatoryAreaId === regulatoryArea?.id ? THEME.color.charcoal : THEME.color.lightGray}
color={
isSubPanelOpened && openPanel.subPanel?.id === regulatoryArea?.id
Copy link
Collaborator

Choose a reason for hiding this comment

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

même question pour isSubPanelOpened

regulatoryAreaId === regulatoryArea?.id
? 'Fermer la zone réglementaire'
: 'Afficher la réglementaire'
isSubPanelOpened && openPanel.subPanel?.id === regulatoryArea?.id
Copy link
Collaborator

Choose a reason for hiding this comment

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

idem

if (layer && layer?.geom && layer?.geom?.coordinates.length > 0) {
const feature = getRegulatoryFeature({ code: Layers.REGULATORY_ENV_PREVIEW.code, layer })
feature.set('metadataIsShowed', true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

pourquoi l'enlever? il permet de mettre un contour plus épais sur la zone qu'on est en train de consulter côté dashboard. C'est dans les maquettes

@maximeperraultdev maximeperraultdev merged commit 3136f5c into main Oct 2, 2024
21 checks passed
@maximeperraultdev maximeperraultdev deleted the maxime/feat/1715/dashboard_amps branch October 2, 2024 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bloc zones AMP. ⏳: 3
3 participants