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

Adds draft SegmentedControl docs #2081

Merged
merged 14 commits into from
May 26, 2022
Merged

Adds draft SegmentedControl docs #2081

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c906d65
adds a draft for the SegmentedControl component docs
mperrotti May 18, 2022
5f85457
Merge branch 'main' into mp/segmented-control-docs-draft
mperrotti May 19, 2022
b8421bd
addresses first round of PR feedback
mperrotti May 19, 2022
bda37f3
Merge branch 'main' of github.com:primer/react into mp/segmented-cont…
mperrotti May 19, 2022
b53ad39
Merge branch 'mp/segmented-control-docs-draft' of github.com:primer/r…
mperrotti May 19, 2022
2947400
Merge branch 'main' into mp/segmented-control-docs-draft
mperrotti May 20, 2022
ffae4ae
Merge branch 'main' into mp/segmented-control-docs-draft
mperrotti May 23, 2022
c754dfd
changes 'block' prop name to 'fullWidth
mperrotti May 23, 2022
6edd979
Update docs/content/SegmentedControl.mdx
mperrotti May 24, 2022
266bce9
Update docs/content/SegmentedControl.mdx
mperrotti May 24, 2022
07d1382
Update docs/content/SegmentedControl.mdx
mperrotti May 24, 2022
0272ead
addresses remainder of PR feedback
mperrotti May 25, 2022
b0b7848
Merge branch 'main' into mp/segmented-control-docs-draft
mperrotti May 25, 2022
33a9474
Merge branch 'main' into mp/segmented-control-docs-draft
mperrotti May 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
adds a draft for the SegmentedControl component docs
  • Loading branch information
@mperrotti
mperrotti committed May 18, 2022
commit c906d65354836b8832b8947c154c880d02fd664c
190 changes: 190 additions & 0 deletions docs/content/SegmentedControl.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
---
title: SegmentedControl
status: Draft
description: Used to select an option from a short list and immediately apply the selection
mperrotti marked this conversation as resolved.
Show resolved Hide resolved
---
mperrotti marked this conversation as resolved.
Show resolved Hide resolved

## Examples

### Simple

```jsx live drafts
<SegmentedControl aria-label="File view">
Copy link
Contributor

Choose a reason for hiding this comment

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

How would you indicate which Button is selected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By passing selected to the SegmentedControl.Button

Copy link
Contributor

Choose a reason for hiding this comment

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

If it'll be common for consumers to explicitly set the selected prop, could we show that in the examples? Similar to how aria-current is set in most of the NavList examples: https://primer-d954104e87-13348165.drafts.github.io/NavList

<SegmentedControl.Item>Preview</SegmentedControl.Item>
Copy link
Member

Choose a reason for hiding this comment

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

How do you feel about <SegmentedControl.Button>? instead of Item

<SegmentedControl.Item>Raw</SegmentedControl.Item>
<SegmentedControl.Item>Blame</SegmentedControl.Item>
</SegmentedControl>
```

### With icons and labels

```jsx live drafts
<SegmentedControl aria-label="File view">
<SegmentedControl.Item leadingIcon={EyeIcon}>Preview</SegmentedControl.Item>
Copy link
Member

Choose a reason for hiding this comment

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

nice! leadingIcon feels like Button's leadingIcon

<SegmentedControl.Item leadingIcon={FileCodeIcon}>Raw</SegmentedControl.Item>
<SegmentedControl.Item leadingIcon={PeopleIcon}>Blame</SegmentedControl.Item>
</SegmentedControl>
```

### With icons only

```jsx live drafts
<SegmentedControl aria-label="File view">
<SegmentedControl.IconItem icon={EyeIcon} aria-label="Preview" />
<SegmentedControl.IconItem icon={FileCodeIcon} aria-label="Raw" />
<SegmentedControl.IconItem icon={PeopleIcon} aria-label="Blame" />
</SegmentedControl>
```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternate idea:

<SegmentedControl aria-label="File view" hideLabels>
  <SegmentedControl.Item leadingIcon={EyeIcon}>Preview</SegmentedControl.Item>
  <SegmentedControl.Item leadingIcon={FileCodeIcon}>Raw</SegmentedControl.Item>
  <SegmentedControl.Item leadingIcon={PeopleIcon}>Blame</SegmentedControl.Item>
</SegmentedControl>

Copy link
Member

@siddharthkp siddharthkp May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

How do you feel about using aria-label instead of hideLabels

<SegmentedControl aria-label="File view">
  <SegmentedControl.Item leadingIcon={EyeIcon} aria-label="Preview" />
  <SegmentedControl.Item leadingIcon={FileCodeIcon} aria-label="Raw" />
  <SegmentedControl.Item leadingIcon={PeopleIcon} aria-label="Blame" />
</SegmentedControl>

alt: similar to IconButton:

<SegmentedControl aria-label="File view">
  <SegmentedControl.IconButton icon={EyeIcon} aria-label="Preview" />
  <SegmentedControl.IconButton icon={FileCodeIcon} aria-label="Raw" />
  <SegmentedControl.IconButton icon={PeopleIcon} aria-label="Blame" />
</SegmentedControl>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm good with

<SegmentedControl aria-label="File view">
  <SegmentedControl.IconButton icon={EyeIcon} aria-label="Preview" />
  <SegmentedControl.IconButton icon={FileCodeIcon} aria-label="Raw" />
  <SegmentedControl.IconButton icon={PeopleIcon} aria-label="Blame" />
</SegmentedControl>

I think the only difference between that and what I have now is the name IconButton instead of Item


### With labels hidden on smaller viewports

```jsx live drafts
<SegmentedControl aria-label="File view" hideLabels={{narrow: false, large: true}}>
<SegmentedControl.Item leadingIcon={EyeIcon}>Preview</SegmentedControl.Item>
<SegmentedControl.Item leadingIcon={FileCodeIcon}>Raw</SegmentedControl.Item>
<SegmentedControl.Item leadingIcon={PeopleIcon}>Blame</SegmentedControl.Item>
</SegmentedControl>
```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternate idea:

<SegmentedControl aria-label="File view" variant={{narrow: 'hideLabels', regular: 'none'}}>
  <SegmentedControl.Item leadingIcon={EyeIcon}>Preview</SegmentedControl.Item>
  <SegmentedControl.Item leadingIcon={FileCodeIcon}>Raw</SegmentedControl.Item>
  <SegmentedControl.Item leadingIcon={PeopleIcon}>Blame</SegmentedControl.Item>
</SegmentedControl>


### Convert to a dropdown on smaller viewports

```jsx live drafts
<SegmentedControl aria-label="File view" asDropdown={{narrow: true, large: false}}>
<SegmentedControl.Item leadingIcon={EyeIcon}>Preview</SegmentedControl.Item>
<SegmentedControl.Item leadingIcon={FileCodeIcon}>Raw</SegmentedControl.Item>
<SegmentedControl.Item leadingIcon={PeopleIcon}>Blame</SegmentedControl.Item>
</SegmentedControl>
```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternate idea:

<SegmentedControl aria-label="File view" variant={{narrow: 'dropdown', regular: 'none'}}>
  <SegmentedControl.Item leadingIcon={EyeIcon}>Preview</SegmentedControl.Item>
  <SegmentedControl.Item leadingIcon={FileCodeIcon}>Raw</SegmentedControl.Item>
  <SegmentedControl.Item leadingIcon={PeopleIcon}>Blame</SegmentedControl.Item>
</SegmentedControl>

Copy link
Member

@siddharthkp siddharthkp May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I like the idea this alternate of using variant for dropdown more!

I think we can have 3 variants: dropdown, compact / without-labels, expanded?? (names TBD 😅)

And then have a responsive format for variant like you mentioned {{narrow: 'dropdown', regular: 'none'}}>

Copy link
Contributor

Choose a reason for hiding this comment

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

What would happen if the variant was "compact" and the buttons didn't have any icons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think anything would happen because there would be no icons to hide


### Fill width of parent

```jsx live drafts
<SegmentedControl block aria-label="File view">
Copy link
Member

Choose a reason for hiding this comment

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

Do we have an example of this already? This seems to come up often, would be nice to have the same prop for it - block / fullWidth

I think in Button, we went in favor of sx={{width: '100%'}} instead of fullWidth, I don't remember the exact reason though cc @pksjce

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with block because that's what I've seen on other components. I'm also fine with sx={{width: '100%'}}

Copy link
Member

@siddharthkp siddharthkp May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I like it, I'm team block/fullWidth :D

Copy link
Contributor

Choose a reason for hiding this comment

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

During our engineering sync, we decided to move forward with a fullWidth prop. @mperrotti will write a brief ADR for this decision.

<SegmentedControl.Item>Preview</SegmentedControl.Item>
<SegmentedControl.Item>Raw</SegmentedControl.Item>
<SegmentedControl.Item>Blame</SegmentedControl.Item>
</SegmentedControl>
```

### In a loading state

```jsx live drafts
<SegmentedControl loading aria-label="File view">
<SegmentedControl.Item>Preview</SegmentedControl.Item>
<SegmentedControl.Item>Raw</SegmentedControl.Item>
<SegmentedControl.Item>Blame</SegmentedControl.Item>
</SegmentedControl>
```

### With a label and caption on the left

```jsx live drafts
<Box display="flex">
<Box flexGrow={1}>
<Text fontSize={2} fontWeight="bold" id="scLabel-vert" display="block">
File view
</Text>
<Text color="fg.subtle" fontSize={1} id="scCaption-vert" display="block">
Change the way the file is viewed
</Text>
</Box>
<SegmentedControl aria-labelledby="scLabel-vert" aria-describedby="scCaption-vert">
<SegmentedControl.Item>Preview</SegmentedControl.Item>
<SegmentedControl.Item>Raw</SegmentedControl.Item>
<SegmentedControl.Item>Blame</SegmentedControl.Item>
</SegmentedControl>
</Box>
```

### With a label above and caption below

```jsx live drafts
<FormControl>
<FormControl.Label id="scLabel-horiz">File view</FormControl.Label>
<SegmentedControl aria-labelledby="scLabel-horiz" aria-describedby="scCaption-horiz">
<SegmentedControl.Item>Preview</SegmentedControl.Item>
<SegmentedControl.Item>Raw</SegmentedControl.Item>
<SegmentedControl.Item>Blame</SegmentedControl.Item>
</SegmentedControl>
<FormControl.Caption id="scCaption-horiz">Change the way the file is viewed</FormControl.Caption>
</FormControl>
Copy link
Member

Choose a reason for hiding this comment

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

Ooh that's interesting! Is this in the context of a Form or otherwise as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise as well. This component is not a substitute for RadioGroup.

Copy link
Member

@siddharthkp siddharthkp May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

👍

The name threw me off a bit 😅

```

### With something besides the first option selected

```jsx live drafts
<SegmentedControl aria-label="File view">
<SegmentedControl.Item>Preview</SegmentedControl.Item>
<SegmentedControl.Item selected>Raw</SegmentedControl.Item>
<SegmentedControl.Item>Blame</SegmentedControl.Item>
</SegmentedControl>
```

### With a selection change handler

```jsx live drafts
<SegmentedControl
aria-label="File view"
onChange={selectedIndex => {
alert(`Segment ${selectedIndex}`)
}}
Copy link
Member

Choose a reason for hiding this comment

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

Because the items are buttons, adding an onClick would also automatically work. Do we want to support both onClick on item and onChange on parent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the onChange on the parent so that we don't have to add repetitive onClick handlers to each SegmentedControl.Button

>
<SegmentedControl.Item>Preview</SegmentedControl.Item>
<SegmentedControl.Item>Raw</SegmentedControl.Item>
<SegmentedControl.Item>Blame</SegmentedControl.Item>
</SegmentedControl>
```

## Props

### SegmentedControl

<PropsTable>
<PropsTableRow name="aria-label" type="string" />
<PropsTableRow name="aria-labelledby" type="string" />
<PropsTableRow name="aria-describedby" type="string" />
<PropsTableRow name="block" type="boolean" description="Whether the control fills the width of its parent" />
<PropsTableRow name="loading" type="boolean" description="Whether the selected segment is being calculated" />
<PropsTableRow
name="onChange"
type="(selectedIndex?: number) => void"
description="The handler that gets called when a segment is selected"
/>
</PropsTable>

### SegmentedControl.Item

<PropsTable>
<PropsTableRow name="aria-label" type="string" />
<PropsTableRow name="leadingIcon" type="Component" description="The leading icon comes before item label" />
<PropsTableRow name="selected" type="boolean" description="Whether the segment is selected" />
</PropsTable>
mperrotti marked this conversation as resolved.
Show resolved Hide resolved

### SegmentedControl.IconItem

<PropsTable>
<PropsTableRow name="aria-label" type="string" />
<PropsTableRow name="icon" type="Component" description="The icon that represents the segmented control item" />
<PropsTableRow name="selected" type="boolean" description="Whether the segment is selected" />
</PropsTable>

## Status

<ComponentChecklist
items={{
propsDocumented: true,
noUnnecessaryDeps: false,
adaptsToThemes: false,
adaptsToScreenSizes: false,
fullTestCoverage: false,
usedInProduction: false,
usageExamplesDocumented: false,
hasStorybookStories: false,
designReviewed: false,
a11yReviewed: false,
stableApi: false,
addressedApiFeedback: false,
hasDesignGuidelines: false,
hasFigmaComponent: false
}}
/>