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

Inconsistent page layout in docs #628

Closed
2 tasks done
pouretrebelle opened this issue Jun 26, 2024 · 5 comments
Closed
2 tasks done

Inconsistent page layout in docs #628

pouretrebelle opened this issue Jun 26, 2024 · 5 comments
Labels
brand documentation Improvements or additions to documentation

Comments

@pouretrebelle
Copy link
Member

pouretrebelle commented Jun 26, 2024

@raytalks has highlighted that the two different layouts for components docs pages can cause confusion, and it's frustrating to have to dig into the React tab to find the component's status labels (maturity and a11y review status)

Unified page Page with tabs

Bento page showing status

Avatar overview page without status
Avatar React page with status

Speaking to @rezrah, highlighting the component labels outside the tabbed area (matching the unified page) would be inappropriate because they refer specifically to the React implementation. In the future each component will also have a Figma tab (matching Primer), so removing the tabbed style and unifying each page is not the solution.

Ideally every component will move towards the tabbed view, where the Overview page covers interface guidelines. This needs to be done in collaboration with Primer Design. I will discuss with them how we can prioritise this work.

Adding interface guidelines to every component would resolve the issue with inconsistent layouts but would bury the component status information deeper within the site, reducing the visibility of this info. I propose we create a component status page (like in primer.style) to highlight the status of each component and its a11y review status.

We could also add a column for whether interface guidelines have been written (and possibly Figma docs?) so we have a clear overview of work that needs to be done for a more mature system, although I appreciate that's not relevant to the implementation so may not be appropriate to include here.

Minor inconsistencies

  • The source and Storybook links being right-aligned in the React tab and left-aligned in the unified view is frustrating. I considered whether we could left-align them in the tab view, but on primer.style the only links in that label bar are in the figma tabs and they're right-aligned, so I wouldn't want to contradict that pattern.
  • On AnchorNav and SubdomainNavBar pages the content is wider than elsewhere, I'll see if a little CSS tweak can fix this.
    Fix content blowout doctocat#739
@pouretrebelle pouretrebelle added brand documentation Improvements or additions to documentation labels Jun 26, 2024
@pouretrebelle
Copy link
Member Author

@emilybrick I hear you're the person to chat about primer.style with! How do you feel about us copying your component status page for Primer Brand? I'm unsure whether it's worth trying to package up the components/styles you use for the status table or whether I can just copy them to this repo, I guess that depends on what your longterm plans are for docs infra. Perhaps we could have a call to check I'm going in the right direction with this docs work?

@emilybrick
Copy link

emilybrick commented Jun 26, 2024

Hey @pouretrebelle!

Love this issue and so happy to see this work happening.

Component page tabs

Some quick backstory - we are planning to update our component pages on primer.style.

  • The "default" (first) tab will include the code examples. Code is the default. Since we no longer need to include Rails in the docs, the default tab will be the PRC components. The name of the default tab is TBD, but it might just become overview.

  • The secondary tab would be guidelines, and this would include interface guidelines

  • Third tab would be Accessibility guidelines


I don't have a mock handy, but essentially:

Overview (code examples, brief description) | Guidelines (interface guidelines) | Accessibility (a11y guidelines)


Component status table

Unfortunately, @dipree and I were thinking of removing the component status table, since we don't think folks are getting a ton of use out of this (or rather, the level of granularity is lost on folks. they just want to know if they can use the component or not).

We need to do more research, but the idea was to instead lean on highlighting the status on the individual component pages. Publicly, on the component pages, it should be Draft or Ready only. Internal states for quality tracking purposes is fine

Happy to chat anytime since there are a lot of moving pieces right now, but also curious if @dipree wants to chime in.

Was there a specific need to showcase the status table on primer.style/brand?

@dipree also wrote this up for more context: https://github.com/github/primer/issues/3317

@pouretrebelle
Copy link
Member Author

pouretrebelle commented Jul 5, 2024

Speaking to Emily earlier this week we've agreed on the path forward - updating the tabs as suggested in her comment, and not pursuing a component status table, but possibly creating some internal reporting to raise visibility of component statuses. Technically this poses some issues though.

Current technical setup for tabbed view

  • There is an MDX file each for the existing index (guidelines) and react tabs
  • They each render a custom layout component which conditionally renders the status, table of contents, and includes a hard-coded tabbed nav (using current page location to determine the active tab)
  • Title and description have to be duplicated in the frontmatter of each MDX file

The drawbacks of this are the content duplication, and having no flexibility of tabs - adding a third tab with this setup would be impossible without a lot more hard-coding and code+content duplication.

Proposed solution

Use a single MDX file for each component, using H1s to define tabs. A custom Gatsby plugin and custom layout component will turn the content into tabs within the page.

Advantages:

  • No duplication of frontmatter
  • Easy to define additional tabs as needed
  • Leveraging Gatsby's in-built table of contents data to build tabs and ToCs
  • No custom components in the MDX to make the layout changes possible, making this content more portable to whatever system we use next
  • Remove the directory hierarchy between component pages with guidelines and those without (e.g. Bento.mdx and Avatar/index.mdx, removing the two levels of directories will make it easier to scan components)

Cons:

  • Each tab won't have its own url, but we can use hash links to link directly to tabs (since we'd be changing the /react path anyway this isn't a huge loss)
    • We'd want the hash links for H2s to still work as expected, so it'll be a bit of work to e.g. jump to the guidelines tab if you load at #usage
  • Need to write some custom Gatsby middleware to parse the page contents and wrap each section in a tab panel. I've done similar things before so it's definitely possible, but may be a bit messy
  • Introducing H1s causes heading hierarchy headaches, because for good a11y each heading should be downgraded a level, but I don't want to mess with their appearance. I think for the MVP I'll leave it with multiple H1s and come back to this later.

I'm going to work on a spike to check the practicality of this solution, but I feel reasonably confident it's the right direction to go in given no viable alternatives. This is a fair bit of work for what would seem like a simple layout switch, but will leave us with a flexible system so a11y guidelines can be slotted in easily, and it could be a testbed for primer.style interface experiments/developments until we do away with Gatsby.

Thoughts/opinions welcome! @raytalks @emilybrick @dipree @rezrah

@raytalks
Copy link

raytalks commented Jul 5, 2024

@pouretrebelle thanks for this! I'd like us to invest time in working together with Primer Engineering to focus on Nextocat rather than making Gatsby work for this use case.
I'm in conversation with @joelhawksley to plan for this work.
In the mean time I'd recommend to focus on low hanging fruit within our Primer Brand documentation and Story book.

@pouretrebelle
Copy link
Member Author

Closing this out as we're not prioritising any significant Gatsby changes since Nextocat is ramping up in priority.
primer/doctocat#739 will be completed though as it's a layout bug rather than UX change.

@pouretrebelle pouretrebelle closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brand documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants