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

Created on 2 November 2024, 14 days ago

Problem/Motivation

UI Patterns 2.x is also replacing Layout Options.

Proposed resolution

  • Create 4 components: grid_row_1, grid_row_2, grid_row_3 & grid_row_4
  • Remove layout_options from info.yml & composer.json
  • Remove ui_suite_dsfr.layout_options.yml and ui_suite_dsfr.layouts.yml
  • Remove template/layout--grid.html.twig
📌 Task
Status

Active

Version

1.0

Component

Code

Created by

🇫🇷France pdureau Paris

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @pdureau
  • 🇫🇷France just_like_good_vibes PARIS

    i will take this one

  • @just_like_good_vibes opened merge request.
  • 🇫🇷France just_like_good_vibes PARIS

    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 %}

  • 🇫🇷France just_like_good_vibes PARIS

    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?

  • 🇫🇷France pdureau Paris

    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.

  • 🇫🇷France just_like_good_vibes PARIS

    changes are done :)

  • 🇫🇷France pdureau Paris

    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...

  • 🇫🇷France just_like_good_vibes PARIS

    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

  • 🇫🇷France just_like_good_vibes PARIS

    let's go

  • 🇫🇷France pdureau Paris

    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

  • 🇫🇷France just_like_good_vibes PARIS

    source of inspiration hahaha :) ?

  • 🇫🇷France pdureau Paris

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

  • 🇫🇷France just_like_good_vibes PARIS

    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

  • 🇫🇷France pdureau Paris

    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

  • 🇫🇷France pdureau Paris

    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]
  • 🇫🇷France pdureau Paris

    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-

  • 🇫🇷France pdureau Paris

    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:

  • @pdureau opened merge request.
  • 🇫🇷France pdureau Paris

    We also need to update grid_row to the new structure

    Ok, let's do it.

  • 🇫🇷France pdureau Paris
  • 🇫🇷France pdureau Paris

    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.

  • 🇫🇷France just_like_good_vibes PARIS

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

  • 🇫🇷France just_like_good_vibes PARIS

    thanks Pierre !

Production build 0.71.5 2024