[1.1.0] From Layout Options to UI Patterns 2.x

i will take this one

i am currently converting, i have several questions :

- do we keep the slot name "main" instead of "first" for slot name? it would help backward compatibility but this is not straightforward to have a column slot named "main" when it is just the first one. i kept the name main for now.
- we originally have a lot of html classes for the grid--layout.html.twig file. i feel like they are not required here. i kept them for now.
- There may be a misunderstanding in the container classes. When i read the doc, it seems like classes fr-container and fr-container--fluid may be exclusive. why having two props then ? i kept two props with_container and fluid_container to have some backward compatibility but updated the twig file to have {% if with_container or fluid_container %}

I found numerous bugs on the actual codebase when trying to render the example page.
therefore i did a few minor adaptations directly in this MR, even if it is not the right place.
You prefer to split into several issues?

Can you rebase?

Also, did you remove template/layout--grid.html.twig?

The changes on fr-icon-* classes would be better in a distinct issue.

do we keep the slot name "main" instead of "first" for slot name? it would help backward compatibility but this is not straightforward to have a column slot named "main" when it is just the first one. i kept the name main for now.

You can chose "first" if you prefer

we originally have a lot of html classes for the grid--layout.html.twig file. i feel like they are not required here. i kept them for now.

The HTML classes need to be compliant with upstream documentation: https://www.systeme-de-design.gouv.fr/fondamentaux/grille-et-points-de-r...

We don't care about the rest. For example, you can skip: 'layout', 'layout--' ~ layout.id|clean_class,

There may be a misunderstanding in the container classes. When i read the doc, it seems like classes fr-container and fr-container--fluid may be exclusive. why having two props then ? i kept two props with_container and fluid_container to have some backward compatibility but updated the twig file to have {% if with_container or fluid_container %}

You are right, a single prop with an enum would be better.

changes are done :)

do we keep the slot name "main" instead of "first" for slot name? it would help backward compatibility but this is not straightforward to have a column slot named "main" when it is just the first one. i kept the name main for now.

I understand the change you did for grid_row_2, grid_row_3 and grid_row_4. But is it expected grid_row_1 as first slot as only slot? Just asking the question...

good question. i think it is simpler to have the same internal names between all the four components... we can keep the label main for the single column case :)

i will correct the labels and repush

let's go

I will take time doing this review because, looking at your props, I may have found a little something to change in UIP2 prop types

source of inspiration hahaha :) ?

(Sidenote for later: why prefixing so much props with "fr_"?)

it was like that in "grid_row" existing component, and in the layout options. so i kept the same naming.

but clearly we can remove the prefixes

This is something we will need to change. And it will be the good opportunity to update grid_row too. But for now, we do nothing until I fixed the UIP2 issue

So, we may need this UIP2 issue to be completed first: 📌 [2.0.0-beta5] Add new EnumSetPropType Active

Because, once coupled with the new SelectsWidget source, the enum_list prop may be a perfect fit for list of values based on the same enum.

For example, those 10 enum props:

    fr_col_first:
      title: "Column span first"
      type: string
      enum: [fr-col-1, fr-col-2, fr-col-3,  fr-col-4, fr-col-5, fr-col-6, fr-col-7, fr-col-8, fr-col-9, fr-col-10, fr-col-11, fr-col-12]
    fr_col_first_sm:
      title: "Column span first (small)"
      type: string
      enum: [fr-col-sm-1, fr-col-sm-2, fr-col-sm-3,  fr-col-sm-4, fr-col-sm-5, fr-col-sm-6, fr-col-sm-7, fr-col-sm-8, fr-col-sm-9, fr-col-sm-10, fr-col-11, fr-col-sm-12]
    fr_col_first_md:
      title: "Column span first (medium)"
      type: string
      enum: [...]
    fr_col_first_lg:
      title: "Column span first (large)"
      type: string
      enum: [...]
    fr_col_first_xl:
      title: "Column span first (extra large)"
      type: string
       enum: [...]
    fr_col_second:
      title: "Column span second"
      type: string
      enum: [fr-col-1, fr-col-2, fr-col-3,  fr-col-4, fr-col-5, fr-col-6, fr-col-7, fr-col-8, fr-col-9, fr-col-10, fr-col-11, fr-col-12]
    fr_col_second_sm:
      title: "Column span second (small)"
      type: string
      enum: [fr-col-sm-1, fr-col-sm-2, fr-col-sm-3,  fr-col-sm-4, fr-col-sm-5, fr-col-sm-6, fr-col-sm-7, fr-col-sm-8, fr-col-sm-9, fr-col-sm-10, fr-col-11, fr-col-sm-12]
    fr_col_second_md:
      title: "Column span second (medium)"
      type: string
      enum: [...]
    fr_col_second_lg:
      title: "Column span second (large)"
      type: string
      enum: [...]
    fr_col_second_xl:
      title: "Column span second (extra large)"
      type: string
      enum: [...]

could be replaced by those 5 enum_list props:

    cols:
      title: Cols
      type: array
      maxItems: 2
      items:
        type: integer
        enum: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
    cols_sm:
      title: Cols SM
      type: array
      maxItems: 2
      items:
        type: integer
        enum: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
    cols_md:
      title: Cols MD
      type: array
      maxItems: 2
      items:
        type: integer
        enum: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
    cols_lg:
      title: Cols LG
      type: array
      maxItems: 2
      items:
        type: integer
        enum: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
    cols_xl:
      title: Cols XL
      type: array
      minItems: 2
      maxItems: 2
      items:
        type: integer
        enum: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]

Side note: on the actual MR (so before my proposal with enum_list, offset values were wrong it some enums fr-col-<code> instead of <code>fr-col-offset-

Proposal based on 📌 [2.0.0-beta5] Add new EnumSetPropType Active : https://git.drupalcode.org/project/ui_suite_dsfr/-/merge_requests/122

If you are OK with this proposal, let's discuss before merging:

• Because 📌 [2.0.0-beta5] Add new EnumSetPropType Active neeeds to be merged before
• We also need to update grid_row to the new structure

We also need to update grid_row to the new structure

Ok, let's do it.

Done: https://git.drupalcode.org/project/ui_suite_dsfr/-/merge_requests/123

If we want to add other attributes props (one by region), like Florent is currently doing with UI Suite Bootstrap, let's do that in a separate issue.

just_like_good_vibes → changed the visibility of the branch 3485201-1.1.0-from-layout to hidden.

thanks Pierre !

Automatically closed - issue fixed for 2 weeks with no activity.