-
Notifications
You must be signed in to change notification settings - Fork 52
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
[doc] Add shape to improve form property section layout #4085
base: master
Are you sure you want to change the base?
Conversation
If `row` and `row-reverse` the "label" div will take `1fr` and the edition div `2fr`. | ||
That will allow to align the "label" div of all the property sections. |
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.
If we uses flexbox, we won't use the fr
unit which comes from grid
. It's either flexbox or grid.
If `row` and `row-reverse` the "label" div will take `1fr` and the edition div `2fr`. | ||
That will allow to align the "label" div of all the property sections. | ||
|
||
The form editor should display the proprty sectiuon accordingly to the chosen `flexDirection` |
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.
section
|
||
== Problem | ||
|
||
A property section in a form is made of a label, a help icon and a edition area. |
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.
Not necessarily an edition area, for exemple LabelWidgetPropertySection
|
||
== Solution | ||
|
||
A new attribute `flexDirection` is added on form widget styles. The value is among `[row, rowReverse, column, columnReverse]`. |
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.
One line per sentence
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.
For the existing exemple CheckboxPropertySection, we didn't use the same naming
labelPlacement: 'end' | 'start' | 'top' | 'bottom';
|
||
A new attribute `flexDirection` is added on form widget styles. The value is among `[row, rowReverse, column, columnReverse]`. | ||
|
||
The label and the help icon are in the same "label" div. The `flexDirection` attribute aims to layout the "label" div and the edition 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.
one line per sentence
A new attribute `flexDirection` is added on form widget styles. The value is among `[row, rowReverse, column, columnReverse]`. | ||
|
||
The label and the help icon are in the same "label" div. The `flexDirection` attribute aims to layout the "label" div and the edition div. | ||
If `row` and `row-reverse` the "label" div will take `1fr` and the edition div `2fr`. |
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.
Look for CheckboxPropertySection
and usage of FormControlLabel
17babde
to
b264bab
Compare
Eventually, grid is not suited because it does not provide the equivalent of row-reverse and column-reverse of Flexbox.
|
b264bab
to
7ae3b29
Compare
The shape may need to be updated to consider Flexbox instead
If you are considering css options for the label, you can assign the label to a specific grid row/column with grid-row and grid-column. If you think you have what you need with flexbox, I just want to see flexbox used from the view DSL to the frontend. Don't try to convert flexbox expressions in the view DSL to grid expressions on the frontend. And if you want to align all the labels of the widgets on one side and then all the widgets, I don't see how you could even do something like that without CSS sub grid. I've removed my positive review and I'll wait for you to complete this shape once you have completely thought this through |
Signed-off-by: Laurent Fasani <[email protected]>
7ae3b29
to
7f79190
Compare
Eventually, I opted for flexbox layout. I did not opt for subGrid because I think it is more complicated for a user that will be the specifier who is not an expert of CSS layout. Moreove, the scope of this ticket does not cover the full property section. If using subgrid, I think it would be defined at a layer global to property sections. |
Pull request template
General purpose
What is the main goal of this pull request?
Project management
priority:
andpr:
labels been added to the pull request? (In case of doubt, start with the labelspriority: low
andpr: to review later
)area:
,difficulty:
,type:
)CHANGELOG.adoc
been updated to reference the relevant issues?CHANGELOG.adoc
? (Including changes in the GraphQL API)CHANGELOG.adoc
? For example indoc/screenshots/2022.5.0-my-new-feature.png
Architectural decision records (ADR)
[doc]
?CHANGELOG.adoc
?Dependencies
CHANGELOG.adoc
?CHANGELOG.adoc
?Frontend
This section is not relevant if your contribution does not come with changes to the frontend.
General purpose
Typing
We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).
useMutation<DATA_TYPE, VARIABLE_TYPE>(…)
useQuery<DATA_TYPE, VARIABLE_TYPE>(…)
useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)
useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)
useState<STATE_TYPE>(…)
?.
(if the GraphQL API specifies that a field cannot benull
, do not treat it has potentiallynull
for example)let diagram: Diagram | null = null;
)Backend
This section is not relevant if your contribution does not come with changes to the backend.
General purpose
Architecture
Review
How to test this PR?
Please describe here the various use cases to test this pull request