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

497 use updated add another component #2520

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

davidtrussler
Copy link
Contributor

@davidtrussler davidtrussler commented Jan 28, 2025

Trello

The changes here implement the latest release of the Publishing Components gem (version 51.0.0) that contains the configuration option added to the "Add another" component that allows its use on Mainstream as required. This adds the parameter empty_fields to the component, which, when set to "true" ensures that no form fields are displayed when the component loads if no content is specified. Additionally these changes update the tests for the changed UI.

They also update the button text from "Add another link" to "Add related external link".

The primary differences between this implementation and the current one are shown in the screenshots below.

Scenario Current implementation Latest component version
Loads with no links specified (without JavaScript) Screenshot 2025-01-28 at 14 47 39 Screenshot 2025-01-28 at 14 47 48
Loads with one link specified (without JavaScript) Screenshot 2025-01-28 at 14 49 54 Screenshot 2025-01-28 at 14 50 02
Loads with no links specified (with JavaScript) Screenshot 2025-01-28 at 14 29 12 Screenshot 2025-01-28 at 14 30 30
"Add" button clicked (with JavaScript) Screenshot 2025-01-28 at 14 33 53 Screenshot 2025-01-28 at 14 34 12
Loads with one link specified (with JavaScript) Screenshot 2025-01-28 at 14 37 19 Screenshot 2025-01-28 at 14 37 09
"Add" button clicked (with JavaScript) Screenshot 2025-01-28 at 14 40 30 Screenshot 2025-01-28 at 14 40 42

@davidtrussler davidtrussler marked this pull request as ready for review January 28, 2025 16:29
Copy link
Contributor

@mtaylorgds mtaylorgds left a comment

Choose a reason for hiding this comment

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

A couple of minor comments related to test names.

end
end

should "display 'Delete' buttons and a second set of inputs when 'Add another link' is clicked" do
click_button("Add another link")
should "display a 'Delete' button and a set of inputs when 'Add another link' is clicked" do
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update the test name

Suggested change
should "display a 'Delete' button and a set of inputs when 'Add another link' is clicked" do
should "display a 'Delete' button and a set of inputs when 'Add related external link' is clicked" do

(or, to ease future maintenance burden, don't quote it exactly and just do "display a 'Delete' button and a set of inputs when the add button is clicked")

(We should also ensure all the tests have consistent terminology if we change it.)

assert page.has_css?("button", text: "Delete")
end
end

should "delete the first set of fields when the user clicks the first “Delete” button" do
click_button("Add another link")
should "delete the set of fields when the user clicks the “Delete” button" do
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be consistent when we're quoting the button text—always use single quotes, or always use double quotes.

Copy link
Contributor

@mtaylorgds mtaylorgds left a comment

Choose a reason for hiding this comment

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

LGTM

- Update `Gemfile.lock` for v.51.0.0 of govuk_publishing_components gem
- Update partial to use updated component
- Update button text
- Update tests for updated UI
@davidtrussler davidtrussler force-pushed the 497_Use-updated-Add-another-component branch from f4e599f to 35b3f61 Compare January 29, 2025 10:07
@davidtrussler davidtrussler merged commit 08d2ad3 into main Jan 29, 2025
13 checks passed
@davidtrussler davidtrussler deleted the 497_Use-updated-Add-another-component branch January 29, 2025 10:15
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