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 heading level 3 for attachments #1914

Merged
merged 3 commits into from
Nov 18, 2020
Merged

Use heading level 3 for attachments #1914

merged 3 commits into from
Nov 18, 2020

Conversation

danacotoran
Copy link
Contributor

Attachments are currently hard coded to use h2 which isn't very flexible. The attachment component is often used as a child of a section of a page, in which case the heading level of the component should be adjusted to reflect the hierarchy of the content.

There is already a h2 in app/views/content_items/_attachments.html.erb – it does not make sense that the attachments have the same heading level as their parent. This specifies that they use h3 instead.

Related PRs:

https://trello.com/c/KzyicaBh/491-publications-confusing-heading-structure


⚠️ This application is Continuously Deployed: ⚠️

  • Merged changes are automatically deployed to staging and production.

  • Make sure you follow the guidance for deployments before you merge.

  • Check your branch is being deployed in the Release app, after merging.

@bevanloon bevanloon temporarily deployed to government-f-attachment-lvdrgy November 16, 2020 13:38 Inactive
@danacotoran danacotoran force-pushed the attachment-heading-level branch from df7f680 to f04e863 Compare November 17, 2020 11:44
@bevanloon bevanloon temporarily deployed to government-f-attachment-lvdrgy November 17, 2020 11:44 Inactive
@chris-gds chris-gds self-requested a review November 17, 2020 14:11
app/views/content_items/_attachments.html.erb Show resolved Hide resolved
<%= render 'govuk_publishing_components/components/attachment',
attachment: @content_item.attachment_details(attachment_id) %>
<%= render 'govuk_publishing_components/components/attachment', {
heading_level: 3,

Choose a reason for hiding this comment

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

Looks fines since you've added to gem, will take a look once on integration.

Choose a reason for hiding this comment

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

Looks good on the gem.

@danacotoran danacotoran force-pushed the attachment-heading-level branch from f04e863 to 737d3f6 Compare November 18, 2020 10:07
@bevanloon bevanloon temporarily deployed to government-f-attachment-lvdrgy November 18, 2020 10:08 Inactive
@danacotoran danacotoran marked this pull request as draft November 18, 2020 10:31
@bevanloon bevanloon temporarily deployed to government-f-attachment-lvdrgy November 18, 2020 10:55 Inactive
@danacotoran danacotoran marked this pull request as ready for review November 18, 2020 11:02
@bevanloon bevanloon temporarily deployed to government-f-attachment-lvdrgy November 18, 2020 11:11 Inactive
@danacotoran danacotoran force-pushed the attachment-heading-level branch from 8e1f82b to 102854c Compare November 18, 2020 11:12
@bevanloon bevanloon temporarily deployed to government-f-attachment-lvdrgy November 18, 2020 11:12 Inactive
@maxgds maxgds merged commit 50b6466 into master Nov 18, 2020
@maxgds maxgds deleted the attachment-heading-level branch November 18, 2020 14:10
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.

4 participants