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

Migrate DS to version 72.2.0 #1579

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Migrate DS to version 72.2.0 #1579

wants to merge 10 commits into from

Conversation

petechd
Copy link
Contributor

@petechd petechd commented Jan 14, 2025

What is the context of this PR?

This migrates Runner to version 72 of the Design System. Adds the latest changes to the html structure for view submitted response, errors and summaries templates.

How to review

Run local instance of the application and check if templates are as expected.

Checklist

  • New static content marked up for translation
  • Newly defined schema content included in eq-translations repo

@petechd petechd marked this pull request as ready for review January 16, 2025 08:22
@petechd petechd requested a review from a team as a code owner January 16, 2025 08:22
Copy link
Contributor

@berroar berroar left a comment

Choose a reason for hiding this comment

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

Noticed some spacing differences when compared with Staging, but guessing they are expected @rmccar?

@rmccar
Copy link
Contributor

rmccar commented Jan 22, 2025

Noticed some spacing differences when compared with Staging, but guessing they are expected @rmccar?

There are some spacing changes included in v72 with the new spacing grid changes so there probably would be some but would be good to go through the changes you've noticed just to be sure

Copy link
Contributor

@berroar berroar left a comment

Choose a reason for hiding this comment

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

Have noticed some weirdness on summary pages, this for example when viewing a summary page in the supplementary data schema.

Looks like the list name is being appended to the row 🤔

Screenshot 2025-01-23 at 13 33 00

@berroar
Copy link
Contributor

berroar commented Jan 24, 2025

Have noticed some weirdness on summary pages, this for example when viewing a summary page in the supplementary data schema.

Looks like the list name is being appended to the row 🤔

Screenshot 2025-01-23 at 13 33 00

Having checked the above, it looks like this issue pops up when we have a list summary for a given section but we don't specify an item_label e.g. the config one from the example above.

"summary": {
                "show_on_completion": true,
                "items": [
                    {
                        "type": "List",
                        "for_list": "products",
                        "title": "Products",
                        "empty_list_text": "There are no products"
                    }
                ],
                "show_non_item_answers": true
            },

Example of the same scenario with the item_label configured:
Screenshot 2025-01-24 at 09 41 31

Setting an item_label is optional and a design choice however, so it looks like something needs to be fixed.

@rmccar
Copy link
Contributor

rmccar commented Jan 24, 2025

Have noticed some weirdness on summary pages, this for example when viewing a summary page in the supplementary data schema.
Looks like the list name is being appended to the row 🤔
Screenshot 2025-01-23 at 13 33 00

Having checked the above, it looks like this issue pops up when we have a list summary for a given section but we don't specify an item_label e.g. the config one from the example above.

"summary": {
                "show_on_completion": true,
                "items": [
                    {
                        "type": "List",
                        "for_list": "products",
                        "title": "Products",
                        "empty_list_text": "There are no products"
                    }
                ],
                "show_non_item_answers": true
            },

Example of the same scenario with the item_label configured: Screenshot 2025-01-24 at 09 41 31

Setting an item_label is optional and a design choice however, so it looks like something needs to be fixed.

Ok great cheers for this, thats really useful to know, I can take this and test it in the DS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants