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

Update Data docs based on feedback #3733

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update Data docs based on feedback #3733

wants to merge 2 commits into from

Conversation

taysea
Copy link
Collaborator

@taysea taysea commented Feb 20, 2024

Deploy Preview

What does this PR do?

Some light touch follow ons based on feedback to:

  • Refine wording to be less developer centric
  • Show code snippets in Toolbar to convey these controls are intended to be in Toolbar components majority of time.
  • Add cross links/related content

Where should the reviewer start?

What testing has been done on this PR?

In addition to the feature you are implementing, have you checked the following:

General UX Checks

  • Small, medium, and large screen sizes
  • Cross-browsers (FireFox, Chrome, and Safari)
  • Light & dark modes
  • All hyperlinks route properly

Accessibility Checks

  • Keyboard interactions
  • Screen reader experience
  • Run WAVE accessibility plugin (Chrome)

Code Quality Checks

  • Console is free of warnings and errors
  • Passes E2E commit checks
  • Visual snapshots are reasonable

How should this be manually tested?

Any background context you want to provide?

What are the relevant issues?

Screenshots (if appropriate)

Should this PR be mentioned in Design System updates?

Is this change backwards compatible or is it a breaking change?

@@ -12,10 +12,10 @@ import {
ToolbarExample,
} from '../../examples/components/';

Data is the wrapper component and orchestrator for a set of subcomponents. Because this set of components work together to create the entirety of the experience, we like to refer to them as "Data and friends".
Because this set of components work together to create the entirety of the experience, we like to refer to them as "Data and friends".
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like Im asking myself "what set of components" while reading this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should I maybe keep the first sentence, but just say "Data is the orchestrator for a set of subcomponents." (just cut out the "wrapper" bit)

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah.. I think just the beginning is what threw me off..
maybe yeah starting with "Data is the orchestrator for a set of subcomponents. These components work together to create the entirety of the experience, we like to refer to them as "Data and friends"."

Copy link
Collaborator

Choose a reason for hiding this comment

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

^ I agree the beginning also threw me off, felt like it needed a little more of an intro

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated this

@taysea taysea requested a review from britt6612 April 2, 2024 15:10
Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Maybe unrelated to this PR but when I filter in the first example on the Data page the pagination seems incorrect. I would expect the pagination controls to not show and the summary should say 'Showing 1-6 of 21 items'

Screen Shot 2024-04-02 at 1 50 30 PM

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.

None yet

3 participants