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

Apply design changes to proposal details #726

Merged
merged 5 commits into from
Sep 1, 2022

Conversation

allbecauseyoutoldmeso
Copy link
Contributor

@allbecauseyoutoldmeso allbecauseyoutoldmeso commented Aug 17, 2022

Description of change

  • Change styling of anchor links
  • Update copy
  • Allow filtering by 'auto answered'
  • Add 'back to top' links

Story Link

https://trello.com/c/3T7wafsB/1110-support-users-to-better-understand-proposal-detail-groups

Screenshot

Screenshot 2022-08-17 at 16 09 15

@allbecauseyoutoldmeso allbecauseyoutoldmeso self-assigned this Aug 17, 2022
@allbecauseyoutoldmeso allbecauseyoutoldmeso force-pushed the kg-proposal-details branch 7 times, most recently from 0052f9a to 4bfd988 Compare August 17, 2022 14:31
end
end

private
def proposal_detail_groups_with_numbers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proposal details have to be numbered before they are filtered, so that if question 2 is filtered out, question 3 is still question 3!

@@ -1,7 +1,7 @@
<div class="govuk-accordion__section">
<div class="govuk-accordion__section-header">
<h3 class="govuk-accordion__section-heading">
<button type="button" id="accordion-default-heading-2" aria-controls="accordion-default-heading-3" class="govuk-accordion__section-button" aria-expanded="false">
<button type="button" id="accordion-default-heading-3" aria-controls="accordion-default-heading-3" class="govuk-accordion__section-button" aria-expanded="false">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this was a typo. accordion-default-heading-2 is the ID for the 'Site map' accordion as well. That uses it consistently, whereas this is mainly using accordion-default-heading-3.

<path fill="currentColor" d="M6.5 0L0 6.5 1.4 8l4-4v12.7h2V4l4.3 4L13 6.4z"></path>
</svg>
<p><%= t(".back_to_top") %></p>
<% end %>
Copy link
Contributor Author

@allbecauseyoutoldmeso allbecauseyoutoldmeso Aug 17, 2022

Choose a reason for hiding this comment

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

There's currently no 'Back to top' link in the gov UK design system, although there's one used on their site, and an open issue to discuss it.

I've put this in a shared partial so we can use it as needed until, hopefully, an official one is added!

@allbecauseyoutoldmeso allbecauseyoutoldmeso marked this pull request as ready for review August 17, 2022 15:28
@allbecauseyoutoldmeso allbecauseyoutoldmeso force-pushed the kg-proposal-details branch 5 times, most recently from 51c18d5 to 515e51f Compare August 25, 2022 09:35
@@ -279,7 +279,7 @@ jobs:
env:
DATABASE_URL: postgres://postgres:postgres@localhost:5432/test
RAILS_ENV: test
SPEC_OPTS: "-f doc -P spec/{helpers,presenters,mailer,jobs,services}/**/*_spec.rb"
SPEC_OPTS: "-f doc -P spec/{helpers,presenters,mailer,jobs,services,translations}/**/*_spec.rb"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just spotted that we're not actually running the specs for i18n configuration in github actions 😬

Copy link
Contributor

@benbaumann95 benbaumann95 left a comment

Choose a reason for hiding this comment

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

Hey this looks really great, sorry it's taken long to look at 🎉 .

I still think I would benefit if we went through this quickly together though at some point just to help my understanding of certain decisions if that's ok?

@allbecauseyoutoldmeso
Copy link
Contributor Author

Hey this looks really great, sorry it's taken long to look at 🎉 .

I still think I would benefit if we went through this quickly together though at some point just to help my understanding of certain decisions if that's ok?

Yes of course...this was a painful piece of work so grateful for other eyes on it! Any time today works for me I think.

@allbecauseyoutoldmeso allbecauseyoutoldmeso force-pushed the kg-proposal-details branch 5 times, most recently from 86046ac to 74a3b1d Compare September 1, 2022 12:10
@allbecauseyoutoldmeso allbecauseyoutoldmeso force-pushed the kg-proposal-details branch 2 times, most recently from c344bb1 to 9a83bda Compare September 1, 2022 14:16
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