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.

Sign up for GitHub

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

Jump to bottom

DPTOPICS-1184/update docs to reflect shift from dl to ul #169

Merged
merged 10 commits into from
Jun 29, 2022
Merged

DPTOPICS-1184/update docs to reflect shift from dl to ul #169

merged 10 commits into from
Jun 29, 2022

Conversation

loki-bbc
Copy link
Contributor

@loki-bbc loki-bbc commented Jun 14, 2022 •
edited
Loading

Ticket: https://jira.dev.bbc.co.uk/browse/DPTOPICS-1184
Slack thread (for more context): #help-webcore-ux

Following the changes made to to support Metadata Snippets without a title (web PR), this PR updates the associated GEL documentation with said component and respective changes.

These doc changes would eventually be visible at https://bbc.github.io/gel/components/metadata-strips/
@loki-bbc loki-bbc marked this pull request as ready for review June 23, 2022 13:20
JoshTumath
JoshTumath approved these changes Jun 23, 2022
View reviewed changes
Copy link
Contributor

@JoshTumath JoshTumath left a comment •
edited
Loading

Choose a reason for hiding this comment

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

EDIT: Sorry, I thought I was reviewing a different PR. 😬
JoshTumath
JoshTumath reviewed Jun 24, 2022
View reviewed changes
Copy link
Contributor

@JoshTumath JoshTumath left a comment

Choose a reason for hiding this comment

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

OK, here's an actual review. 😅 I think it looks good. I've never reviewed a GEL tech doc change before, so you may want a second opinion.
_content/components/metadata-strips.md Outdated Show resolved Hide resolved
_content/components/metadata-strips.md Outdated
[^1]: Don't make every `<svg>` focusable by default (issue) — Microsoft, <https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8090208/>
[^2]: "Accessibility chops: `hidden` and `aria-hidden`" — The Paciello Group, <https://developer.paciellogroup.com/blog/2012/05/html5-accessibility-chops-hidden-and-aria-hidden/>
[^3]: Use Of Color: Understanding WCAG 1.4.1, <https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-without-color.html>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update this link to the WCAG 2.1 guidelines? https://www.w3.org/WAI/WCAG21/Understanding/use-of-color
sbeaumontweb
sbeaumontweb reviewed Jun 24, 2022
View reviewed changes
Copy link
Collaborator

@sbeaumontweb sbeaumontweb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Heyo, could we update the version on this component to 1.0.0? I'd call this significant changes to this piece 😄 Will merge once done, cheers!
@loki-bbc
Copy link
Contributor Author

loki-bbc commented Jun 27, 2022 •
edited
Loading

Heyo, could we update the version on this component to 1.0.0? I'd call this significant changes to this piece 😄 Will merge once done, cheers!

No problems, thank you for the review have done that! As we're still awaiting an articles review on the web PR (https://github.com/bbc/web/pull/8173), and for our testers to comb through it, maybe we hold fire on merging this until we're ready to merge that first? I don't anticipate there being any changes needed that would need to be reflected in a doc change, but just a precautionary measure.

Edit: I tried adding a DO NOT MERGE label, but it seems I can't add labels to this PR? Maybe another permissions thing, but I would've thought a fork would let me. I've edited the title of the PR just now to reflect this. I'll ping someone once the other PR is ready to be merged.
@loki-bbc loki-bbc changed the title DPTOPICS-1184/update docs to reflect shift from dl to ul Jun 27, 2022
@loki-bbc loki-bbc changed the title DON'T MERGE YET: DPTOPICS-1184/update docs to reflect shift from dl to ul Jun 28, 2022
@sbeaumontweb sbeaumontweb merged commit c48fa00 into bbc:master Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants
@loki-bbc @JoshTumath @sbeaumontweb