Updates to grouped FAQs by rezrah · Pull Request #410 · primer/brand

rezrah · 2023-08-30T09:40:46Z

Summary

Updates appearance for displaying grouped FAQ content, to match recommendations from Site Refactor designs.

🔗 Storybook examples
🔗 Documentation

Part of https://github.com/github/primer/issues/2510

List of notable changes:

added a new FAQGroup component to wrap individual FAQ children.
- Aligns with the approach used for ButtonGroup
updated Accordion to support new variant prop, which now supports: default and emphasis formats that have unique visuals
updated Accordion toggles to be end aligned by default, and optionally start align using a reversedToggles prop
updated FAQ subheading styles (smaller and muted)
fixed nested accordions, which previously didn't work

What should reviewers focus on?

Try it out in Storybook and Documentation
Providing feedback on the new API

Steps to test:

Go to this Storybook story
Check the design matches the proposal in light and dark mode

Supporting resources (related issues, external links, etc):

Figma design file

Contributor checklist:

All new and existing CI checks pass
Tests prove that the feature works and covers both happy and unhappy paths
Any drop in coverage, breaking changes or regressions have been documented above
New visual snapshots have been generated / updated for any UI changes
All developer debugging and non-functional logging has been removed
Related issues have been referenced in the PR description

Reviewer checklist:

Check that pull request and proposed changes adhere to our contribution guidelines and code of conduct
Check that tests prove the feature works and covers both happy and unhappy paths
Check that there aren't other open Pull Requests for the same update/change

Screenshots:

Please try to provide before and after screenshots or videos

Light	Dark

Narrow viewport presentation:

changeset-bot · 2023-08-30T09:40:49Z

🦋 Changeset detected

Latest commit: 63b7545

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@primer/react-brand	Minor
@primer/brand-primitives	Minor
@primer/brand-e2e	Minor
@primer/brand-fonts	Minor
@primer/brand-config	Minor
@primer/brand-storybook	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-08-30T09:43:50Z

🔍 Design token changes found

View CSS variable changes

- --brand-FAQ-maxWidth: 51rem;
+ --brand-FAQ-maxWidth-list: 51rem;

- --brand-FAQ-heading-color: var(--brand-color-text-default);
+ --brand-FAQ-color-heading: var(--brand-color-text-default);

- --brand-FAQ-heading-color: var(--brand-color-text-default);
+ --brand-FAQ-color-heading: var(--brand-color-text-default);

- --brand-FAQ-heading-color: var(--brand-color-text-default);
+ --brand-FAQ-color-tabBg-selected: var(--base-color-scale-blue-0);

github-actions · 2023-08-30T09:52:17Z

🟢 No visual differences found

Our visual comparison tests did not find any differences in the UI.

…abbed-group

jesussandreas · 2023-08-31T17:21:59Z

looking good @rezrah.

minor comments:

the border color on the lines within the faq. i know it's from one on the system, could we use a custom color one? suggesting light/gray/02.
let's proceed and remove the border on the hover effect tabs. updated figma so it reflects this.

other than that. looks great. thank you for putting this together! ❤️

josepmartins

Lgtm!

Some comments around color usage and styles based on the Figma designs @jesussandreas

Should the space between the heading and the navigation+question be 112 in large viewports and 64px in mobile? Right now is a fixed 64px value

Are the + icons supposed to be blue by default?

Dividers/borders in the mobile version are black/default color. Should we change them?

In the figma files, the FAQ group doesn't have the subheading repeated in the right list, did that change or is the figma outdated?

josepmartins · 2023-09-01T08:20:35Z

+        },
+        "tabBg": {
+          "selected": {
+            "value": "hsl(var(--base-color-scale-blue-5-hsl) / 08%)",


In pro of using solid colors, could this be --base-color-scale-blue-0? cc @jesussandreas

Using transparency over that shade of blue causes the color to be a bit more greyish than the normal saturated blue

Blue 5 with transparency Solid blue 0

yeah, that works!

josepmartins · 2023-09-01T08:24:00Z

+        "tabBg": {
+          "selected": {
+            "value": "hsl(var(--base-color-scale-blue-5-hsl) / 08%)",
+            "dark": "hsl(var(--base-color-scale-blue-4-hsl) / 08%)"


Same here, we could use blue 9 instead

@jesussandreas @josepmartins -- I'm confused. In our Enterprise comp, we're not using a blue highlight at all. Can we make sure that we're following the patterns outlined in our Landing Pages so there's no discrepancies?

i think it's because initially the blue, it was thought to act as the accent color (ex: our default pink) that can change depending on the brand theme. but i can double check on this and perhaps we blue as our set color.

@jesussandreas im mostly referring to the left hand groups, not the accordion icons. I worry about using brand themes to impact active states. The blue makes sense here, but I'd love to have a gray default set here in case we want to reduce the volume.

I added that to the tracking issue https://github.com/github/primer/issues/2510, as this PR was merged last week so we need to update Figma and React libraries.

rezrah · 2023-09-04T12:44:02Z

@josepmartins - thanks for creating the Figma version of this pattern. Addressing a few points before I merge this:

In the figma files, the FAQ group doesn't have the subheading repeated in the right list, did that change or is the figma outdated?

These are from the enterprise designs, which are supposedly the latest representations of each components.

Are the + icons supposed to be blue by default?

I've kept those as accent values, so we can theme the LPs more easily. I'd rather keep these as our accent color unless there's a good reason to change it globally to blue.

Should the space between the heading and the navigation+question be 112 in large viewports and 64px in mobile? Right now is a fixed 64px value

FWIW on Enterprise designs i'm seeing 80 in larger viewports so 🤷

I think this is fairly easy to fix later, so going to proceed with the initial release of this pattern for now. cc. @jesussandreas @ajashams

ajashams · 2023-09-05T20:59:55Z

@rezrah @josepmartins -- Apologies on our delay. Just confirming here for posterity:

Yes to using accent on plus icons
I see we added the sub-heading to the top of the accordions which wasn't designed that way, but could prove to be clarifying for users. If we move forward with that, can we make sure that the baseline of sub-header is horizontally-aligned with the Section text? (see image below)
Dividers between questions should be the same border color we use on both light (#B7BFC7) and dark (#484F58)
Dividers on mobile between FAQ sections should be #000
Im also noticing that there's a weird 12px top-padding applied to the mobile view of the section accordions (see image) which feels off. Can we remove that and maintain a 24px top- and bottom-padding between accordions?

Sub-header text alignment	24px padding between accordions

mperrotti and others added 4 commits August 14, 2023 11:42

supports tabbed faq groups, starts on small screen variant

9bf5654

fix merge conflict

bc3b62b

shoring up FAQ group components

eaf8f10

update snapshots

c2f616e

rezrah requested review from JoshBowdenConcepts, ajashams, danielguillan, jesussandreas, josepmartins and mperrotti August 30, 2023 09:40

add changeset

4b042eb

rezrah temporarily deployed to github-pages August 30, 2023 09:49 — with GitHub Actions Inactive

rezrah temporarily deployed to github-pages August 30, 2023 10:01 — with GitHub Actions Inactive

add subheaidng to tab group

4903a3d

rezrah temporarily deployed to github-pages August 30, 2023 18:00 — with GitHub Actions Inactive

rezrah added 2 commits August 31, 2023 14:51

Merge branch 'main' of github.com:primer/react-brand into rezah/faq-t…

d4e859b

…abbed-group

update snapshot

ca62514

rezrah temporarily deployed to github-pages August 31, 2023 14:13 — with GitHub Actions Inactive

rezrah added 2 commits August 31, 2023 15:46

add tests

0ef5927

fix changeset typo

685b37f

rezrah temporarily deployed to github-pages August 31, 2023 15:03 — with GitHub Actions Inactive

josepmartins approved these changes Sep 1, 2023

View reviewed changes

use solid bgs for tabs

63b7545

rezrah temporarily deployed to github-pages September 4, 2023 13:00 — with GitHub Actions Inactive

rezrah merged commit 8933e19 into main Sep 4, 2023

rezrah deleted the rezah/faq-tabbed-group branch September 4, 2023 13:19

primer-css mentioned this pull request Sep 4, 2023

Release tracking #403

Merged

rezrah mentioned this pull request Sep 7, 2023

FAQ Group visual updates #416

Merged

9 tasks

josepmartins mentioned this pull request Sep 14, 2023

[FAQGroup] Update navigation styles #425

Merged

9 tasks

Uh oh!

Conversation

rezrah commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

List of notable changes:

What should reviewers focus on?

Steps to test:

Supporting resources (related issues, external links, etc):

Contributor checklist:

Reviewer checklist:

Screenshots:

Uh oh!

changeset-bot Bot commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions Bot commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 Design token changes found

Uh oh!

github-actions Bot commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟢 No visual differences found

Uh oh!

jesussandreas commented Aug 31, 2023

Uh oh!

josepmartins left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josepmartins Sep 1, 2023

Choose a reason for hiding this comment

Uh oh!

jesussandreas Sep 8, 2023

Choose a reason for hiding this comment

Uh oh!

josepmartins Sep 1, 2023

Choose a reason for hiding this comment

Uh oh!

ajashams Sep 9, 2023

Choose a reason for hiding this comment

Uh oh!

jesussandreas Sep 11, 2023

Choose a reason for hiding this comment

Uh oh!

ajashams Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

josepmartins Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

rezrah commented Sep 4, 2023

Uh oh!

ajashams commented Sep 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rezrah commented Aug 30, 2023 •

edited

Loading

changeset-bot Bot commented Aug 30, 2023 •

edited

Loading

github-actions Bot commented Aug 30, 2023 •

edited

Loading

github-actions Bot commented Aug 30, 2023 •

edited

Loading

josepmartins left a comment •

edited

Loading