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

[Carte] Normalise la position des menus #3131

Closed
wants to merge 10 commits into from

Conversation

ivangabriele
Copy link
Member

Linked issues


  • Tests E2E (Cypress)

@ivangabriele ivangabriele force-pushed the ivan/normalize-map-menu-margins branch 3 times, most recently from a7574b3 to 3e30ef4 Compare April 23, 2024 21:25

import { mainWindowActions } from '../slice'

export function LeftMenu() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
export function LeftMenu() {
export function LeftMapMenu() {

?

const missionButtonRef = useRef<HTMLDivElement | null>(null)

const dispatch = useMainAppDispatch()
const { favorites } = useMainAppSelector(state => state.favoriteVessel)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const { favorites } = useMainAppSelector(state => state.favoriteVessel)
const favorites = useMainAppSelector(state => state.favoriteVessel.favorites)

?

)}

{import.meta.env.FRONTEND_PRIOR_NOTIFICATION_LIST_ENABLED === 'true' && (
<IconButton Icon={Icon.Fishery} size={Size.LARGE} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il manque d'ailleurs ce bouton dans le slice displayedComponent

Copy link
Member Author

Choose a reason for hiding this comment

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

Bien vu !

left: 112px;
top: 12px;

* {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il faudra qu'on arrive à enlever ce style car on est obligé de l'injecter dans beaucoup de composants enfants :/

Copy link
Member Author

@ivangabriele ivangabriele Apr 25, 2024

Choose a reason for hiding this comment

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

Alors l'idée c'est de finir par le mettre en global plutôt comme c'est le standard pour grandement facilement les calculs de tailles en séparant l'espace intérieur de l'espace extérieur.

Du coup je suis d'accord mais dans le sens où une fois tout migré, on l'aura en global via rsuite et on pourra le retirer partout ailleurs.

Au passage cette PR fixe pas mal de soucis visuels en prod, sur les dialogs/overlays des contacts par exemple.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oui c'est ce que je voulais dire pour le passage en global et le fait de l'enlever dans les composants enfants

{isFavoriteVesselsMapButtonDisplayed && (
<Block>
<IconButtonWrapper>
<IconButtonBadge $isActive={leftDialog?.key === 'FAVORITE_VESSELS'}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

il manque un onClick ?

Copy link
Member Author

Choose a reason for hiding this comment

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

In progress, mais oui en effet.

}

return (
<Wrapper data-cy="missions-menu-box" style={{ top: leftDialog?.topPosition }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Je ne vois pas où est l'animation sur l'ouverture ?

import { sideWindowActions } from '../../../domain/shared_slices/SideWindow'
import { mainWindowActions } from '../../MainWindow/slice'

export function MissionMenuDialog() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ça me va d'utiliser Dialog

@@ -11,6 +13,15 @@ export const addReporting =
(newReporting: ReportingCreation): MainAppThunk<Promise<void>> =>
(dispatch, getState) => {
const { selectedVesselIdentity } = getState().vessel
// TODO Can this case happen? Is it the right way to handle it?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Je ne vois pas comment, car un signalement s'ajoute quand la fiche navire est ouverte (on a donc un selectedVesselIdentity).
Autant mettre un assert je pense

(dispatch, getState) => {
const { selectedVesselIdentity } = getState().vessel
// TODO Can this case happen? Is it the right way to handle it?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

@@ -29,6 +30,15 @@ export function VesselName({ focusOnVesselSearchInput }) {
const addOrRemoveToFavorites = useCallback(
e => {
e.stopPropagation()
// TODO Can this case happen? Is it the right way to handle it?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same, n'est possible que dans la fiche navire est ouverte

@ivangabriele ivangabriele force-pushed the ivan/normalize-map-menu-margins branch from 3e30ef4 to eb25491 Compare May 13, 2024 05:44
Copy link

sonarcloud bot commented May 13, 2024

Quality Gate Passed Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@louptheron
Copy link
Collaborator

Repris ici : #3634

@louptheron louptheron closed this Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech. enhancement technical ehancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants