-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Design] Utilisation des composants Monitor-ui pour remplacer les composants RSuite #1189
[Design] Utilisation des composants Monitor-ui pour remplacer les composants RSuite #1189
Conversation
ac4fb6b
to
10bc810
Compare
a46a5a9
to
a2846f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
joli clean !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plein de comments de forme mais super taff ! Et je suis totalement d'accord avec ton principe "plus d'utilisation de Rsuite directement dans les projets".
Ca permettra aussi de ne plus installer Rsuite ^^.
J'espère qu'on pourra faire ça un jour sur Fish mais ça devra être divisé en plusieurs Prs parce que c'est colossal comme travail.
@@ -19,3 +20,8 @@ export type NewInterestPoint = { | |||
type: string | null | |||
uuid: string | |||
} | |||
|
|||
export type InterestPointOptionValueType = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InterestPointOptionValue
suffit ;). +Type c'est vraiment quand on a pas le choix dans des edge cases complexes.
color: ${p => p.theme.color.gunMetal}; | ||
border-bottom: 1px solid ${p => p.theme.color.lightGray}; | ||
line-height: 1.9em; | ||
const StyledMultiRadio = styled(MultiRadio)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A reporter sur monitor-ui ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c'est un style particulier, avec des bordures et un hover spécifique donc pour moi ça ne va pas dans monitor-ui
@@ -113,7 +112,7 @@ const getShowedCoordinates = (coordinates, coordinatesFormat) => { | |||
const StyledCoordinatesContainer = styled.div` | |||
z-index: 2; | |||
` | |||
const RadioWrapper = styled(RadioGroup)` | |||
const RadioContainer = styled.div` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A reporter sur monitor-ui ? On peut styler directement les fields normalement, styles et className sont sensés être attachés sur leur Field ou Fieldset interne. A corriger sur monitor-ui si ce n'est pas le cas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idem c'est un style particulier à ce composant
frontend/src/features/map/tools/interestPoint/EditInterestPoint.tsx
Outdated
Show resolved
Hide resolved
:not(:last-child) { | ||
margin-right: 6px; | ||
} | ||
` | ||
const FormGroup = styled.div` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je me demande si ça ne vaudrait pas le coup de créer un petit élément partagé sur monitor-ui comme on commence à avoir pas mal de forms avec ce genre de chose qui sont normalisables.
display: flex; | ||
flex-direction: column; | ||
gap: 24px; | ||
padding-bottom: 48px; | ||
` | ||
const StyledToggle = styled.div` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Là aussi : monitor-ui ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
j'ai supprimé le code lié au toggle (il ne servait plus). Mais oui on pourrait mettre ça dans monitor-ui Toggle + texte. je ferais ça dans une autre pr
Related Pull Requests & Issues