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

Use table component on API users application index #2638

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

chrislo
Copy link
Contributor

@chrislo chrislo commented Jan 10, 2024

Trello: https://trello.com/c/wSKG73MP
Arises from: #2599

To ensure we keep up with changes to the design system this PR replaces the table markup on /app/views/api_users/applications/index.html.erb with the table design system component.

Before

Screenshot 2024-01-10 at 15 08 28

After

Screenshot 2024-01-10 at 15 08 13

@chrislo chrislo marked this pull request as ready for review January 10, 2024 15:18
@floehopper floehopper self-assigned this Jan 10, 2024
@floehopper floehopper self-requested a review January 10, 2024 18:07
Copy link
Contributor

@floehopper floehopper left a comment

Choose a reason for hiding this comment

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

I added one very minor question/suggestion, but otherwise this looks good to me!

@floehopper
Copy link
Contributor

Oh, I just noticed the change in size of the table heading/caption. Is that consistent with what we've got elsewhere...?

@chrislo
Copy link
Contributor Author

chrislo commented Jan 11, 2024

Oh, I just noticed the change in size of the table heading/caption. Is that consistent with what we've got elsewhere...?

Yes, I think so - we're using the caption option for the table component in several other places in the code.

I want to convert this table to use the table component. Extracting
this helper first will make that easier.
This ensures the markup generated reflects the latest changes in the
design system. I've decided to retain the `govuk-visually-hidden`
Permissions header and to display an empty cell for the update
permissions link, rather than the "Not set" default[1] when no link is
required.

[1] https://github.com/alphagov/govuk_publishing_components/blob/1df657ae5483552cf735396174c38eed134ba433/spec/components/table_spec.rb#L47
@chrislo chrislo force-pushed the use-table-component-on-api-users-application-index branch from c527c8b to d561fd7 Compare January 11, 2024 10:02
@chrislo chrislo merged commit 7fad431 into main Jan 11, 2024
16 checks passed
@chrislo chrislo deleted the use-table-component-on-api-users-application-index branch January 11, 2024 10:07
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.

2 participants