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

content modelling/899 copy embed code #9942

Merged
merged 5 commits into from
Feb 18, 2025

Conversation

Harriethw
Copy link
Contributor

@Harriethw Harriethw commented Feb 17, 2025

https://trello.com/c/Hr1mmk1C/899-add-copy-embed-code-to-attributes

Screenshot 2025-02-17 at 16 21 12

Adding the 'copy code' to specific attributes on a rate.
This involves first calculating what the embed code is for the nested field, then adding the module to the row in the summary card.

The copy code should only appear next to Amount for MVP, but that can be done in future PR.


⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

this requires an external method to calculate what
the actual field path is, which feels a bit
brittle, but not sure what the solution would be
atm - in future maybe a service or helper that
find the path given the object type and schema?
the 'copy embed code' module only works with the
original govuk component - I don't think there is
any impactful difference, and this allows us to be
up to date with the latest GDS changes.
we will also use this to determine whether to show
the copy code button
@Harriethw Harriethw force-pushed the content-modelling/899-copy-embed-code branch from e08a649 to b98df20 Compare February 17, 2025 16:20
@Harriethw Harriethw requested a review from pezholio February 17, 2025 16:23
@Harriethw Harriethw marked this pull request as ready for review February 17, 2025 16:23
@@ -16,10 +16,18 @@ def title
end

def rows
object_keys_rows
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't think this actually needs to be a new method!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nah, you're right, just move the code from object_keys_rows to here I reckon. Cuts down on the misdirection.

Copy link
Contributor

@pezholio pezholio left a comment

Choose a reason for hiding this comment

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

Looks good so far, just a couple of comments

"#{embed_code_prefix}}}"
end

def embed_code_for_field(field_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is OK for now, but yeah, I think we can probably do something a bit smarter given the schema etc

@@ -16,10 +16,18 @@ def title
end

def rows
object_keys_rows
Copy link
Contributor

Choose a reason for hiding this comment

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

Nah, you're right, just move the code from object_keys_rows to here I reckon. Cuts down on the misdirection.

object_keys_rows
end

def object_keys_rows
object.keys.map do |key|
{
key: key.titleize,
value: object[key],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to have the embed code outputted here too, then hidden when we initialize the copy-embed-code JS module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah I meant to say I'd pick the no-JS option up in another PR!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as the new design is a bit different and I think will mean changes to the actual javascript https://trello.com/c/Hr1mmk1C/899-add-copy-embed-code-to-attributes?filter=label:Dev

so that it should not appear on the `review` page.
@Harriethw Harriethw force-pushed the content-modelling/899-copy-embed-code branch from b98df20 to dfd5983 Compare February 17, 2025 17:00
@Harriethw Harriethw merged commit 3d0d034 into main Feb 18, 2025
19 checks passed
@Harriethw Harriethw deleted the content-modelling/899-copy-embed-code branch February 18, 2025 10:12
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