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

DSRC-65: Call to action block #1648

Merged
merged 19 commits into from
Jun 18, 2024
Merged

DSRC-65: Call to action block #1648

merged 19 commits into from
Jun 18, 2024

Conversation

jamesbiggs
Copy link
Collaborator

@jamesbiggs jamesbiggs commented Jun 12, 2024

Ticket URL: DSRC-65 + DSRC-68

About these changes

Adds a CTA block (DSRC-65) and a button block (DSRC-68)

In the API:
image

In the CMS:
image

Before assigning to reviewer, please make sure you have

  • Checked things thoroughly before handing over to reviewer
  • Checked PR title starts with ticket number as per project conventions to help us keep track of changes
  • Ensured that PR includes only commits relevant to the ticket
  • Waited for all CI jobs to pass before requesting a review
  • Added/updated tests and documentation where relevant

Merging PR guidance

Follow docs\developer-guide\contributing.md

Deployment guidance

Follow docs\infra\environments.md

@jamesbiggs jamesbiggs marked this pull request as draft June 12, 2024 12:59
@jamesbiggs jamesbiggs marked this pull request as ready for review June 13, 2024 09:14
@jamesbiggs jamesbiggs requested a review from ahosgood June 13, 2024 09:47
@ahosgood
Copy link
Member

Was there a requirement for the ability to have multiple CTA buttons next to one another?

We also need a boolean field for the button which is accented, defaulting to False. Setting to True would allow the editor to add an accented (coloured) button.

We also need an optional title field so editors can include a longer, more descriptive title of where the button goes.

@ahosgood
Copy link
Member

Another thought - are these going into article pages or the yet-to-be-defined "standard page"?

@jamesbiggs
Copy link
Collaborator Author

Another thought - are these going into article pages or the yet-to-be-defined "standard page"?

I just had a chat with Lizzy and she said they will be in a "standard" page, so I'll make a PR before this one is merged to add a standard page, add these components to that, and remove them from Article pages.

Was there a requirement for the ability to have multiple CTA buttons next to one another?

Content haven't asked for it... But I think there could be a case for that (at some point). We can always allow this to add multiple buttons at a later point if that's the case 🙂

We also need a boolean field for the button which is accented, defaulting to False. Setting to True would allow the editor to add an accented (coloured) button.

👍

We also need an optional title field so editors can include a longer, more descriptive title of where the button goes.

They have the sub-heading feature at the minute, I think they would use that here instead of having a dedicated CTA "title"?

@jamesbiggs
Copy link
Collaborator Author

image

@ahosgood
Copy link
Member

Was there a requirement for the ability to have multiple CTA buttons next to one another?

Content haven't asked for it... But I think there could be a case for that (at some point). We can always allow this to add multiple buttons at a later point if that's the case 🙂

Could we make it a list with a max of 1? I'm just thinking about the future - it would be more pain to change a dict to a list because the new format would "break" the frontend whereas adding more buttons, it might just not display them.

We also need an optional title field so editors can include a longer, more descriptive title of where the button goes.

They have the sub-heading feature at the minute, I think they would use that here instead of having a dedicated CTA "title"?

I was on about a specific title attribute on the button itself but looking at it more, I don't think this is needed. We'll just need give advice to devolved editors on how to properly write button text. Carry on!

@jamesbiggs jamesbiggs merged commit 65a4eb3 into develop Jun 18, 2024
5 checks passed
@jamesbiggs jamesbiggs deleted the feature/DSRC-65-cta-block branch June 18, 2024 09:04
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