Hidden component #2301

pksjce · 2022-09-02T10:50:10Z

The Hidden component will be used to hide or show content in a responsive manner. We can do this across three viewport breakpoints aka 'narrow', 'regular' and 'wide'.

It is inspired by Braid design system

This was built to satisfy use cases specific to PageHeader eg -

Here, we want to show different text inside the primary Button based on viewport value.

Merge checklist

changeset-bot · 2022-09-02T10:50:13Z

🦋 Changeset detected

Latest commit: 685fdb8

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

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 · 2022-09-02T10:53:19Z

size-limit report 📦

Path	Size
dist/browser.esm.js	77.21 KB (0%)
dist/browser.umd.js	77.85 KB (0%)

joshblack · 2022-09-02T16:23:46Z

docs/content/Hidden.mdx

+---
+
+The `Hidden` component is a `<span />` that will help you hide or show content/components in different viewports.
+**Attention:** Use Hidden only to control behaviour in a responsive manner. It does not take any `sx` props.


Quick question, would there be a way to control block vs inline flow for this component? Or is the intention that it should always be inline?

Does hidden have to render any DOM element? Could it be something like this:

function Hidden({when, children}) { const isHidden = useResponsiveValue(...) return !isHidden ? children : null }

yeah so the tossup was between do a display:none or not render the component at all. I like not rendering. reduces markup + no need to specify display value at all.

colebemis · 2022-09-02T18:27:35Z

docs/content/Hidden.mdx

+    type="string or Array"
+    description="Can be one or more values of 'narrow', 'wide', 'regular'"


Wonder if a more specific definition would be helpful here:

Suggested change

type="string or Array"

description="Can be one or more values of 'narrow', 'wide', 'regular'"

type={`

| 'narrow'

| 'regular'

| 'wide'

| Array<'narrow' | 'regular' | 'wide'>`}

description="The viewport range(s) at which children should be hidden"

Yeah, I could write the actual size values.

siddharthkp · 2022-09-05T07:22:52Z

Hi 👋

Should we start this component in drafts till we feel confident about our responsive story?

pksjce · 2022-09-08T07:16:08Z

Closing this PR.
Basically after discussion with @siddharthkp and reviewing Chakra library, I think it's better we use styled system's responsive props to power this hide behaviour. I will update the API docs accordingly.

pksjce · 2022-09-15T05:39:50Z

After some more thought and trying out the alternative sx way of having responsive css, I'm somewhat gravitating back to the simplicity and utility of this component.
I have addressed existing feedback and moved this component to the drafts folder. I think we should test out the utility before writing off this component.

…ontent

joshblack

Love this component! Left some comments and questions below, let me know if they make no sense 😅 haha

joshblack · 2022-09-16T20:47:20Z

src/Hidden/Hidden.tsx

+  children: React.ReactNode
+}
+
+export const Hidden = ({on: hiddenViewports, children}: HiddenProps) => {


I loved the when convention from the docs, it was nice to read through it and think: "Okay, this is hidden when the viewport is narrow and wide". Is there a mental scheme you'd recommend with on?

I kept going back and forth with when and on in my head. Sorry for the mix-up. Vini was also leaning towards when. Will change to that.
For me its just a shorter sentence to say hidden on narrow viewport vs hidden when viewport is narrow. When I think of it, I feel like its a non-native english speaker thing. 🤣

joshblack · 2022-09-16T20:50:01Z

src/Hidden/Hidden.tsx

+  const isNarrowViewport = useMedia(viewportRanges.narrow)
+  const isRegularViewport = useMedia(viewportRanges.regular)
+  const isWideViewport = useMedia(viewportRanges.wide)


Do you think it'd be possible to generate one media query string from the given viewports? Would be cool to do something like:

function mediaQuery(rangeOrRanges: Array<Viewports> | Viewports) { const ranges = Array.isArray(rangeOrRanges) ? rangeOrRanges : [rangeOrRanges] return ranges.map(range => viewportRanges[range]).join(', ') } export const Hidden = ({on: hiddenViewports, children}: HiddenProps) => { const matches = useMedia(mediaQuery(hiddenViewports)) if (matches) { return null } return children }

Basically any time one of the media queries from on is true, we know it should hidden, otherwise return the children.

Yeah, its a good optimisation!

joshblack · 2022-09-16T20:50:24Z

src/Hidden/Hidden.tsx

+  children: React.ReactNode
+}
+
+export const Hidden = ({on: hiddenViewports, children}: HiddenProps) => {


Would we need to worry about providing a default value for SSR in this exploration?

joshblack · 2022-09-16T20:51:06Z

docs/content/drafts/Hidden.mdx

+## Example
+
+```jsx live
+<Hidden when="narrow">


Based on the implementation below, will this be changing to on?

siddharthkp

Let's ship it and use it internally to get a better sense of how it feels :)

(the approach I wanted to try with sx doesn't even work yet, so that's my bad!)

pksjce self-assigned this Sep 2, 2022

pksjce requested review from colebemis and siddharthkp September 2, 2022 10:53

pksjce marked this pull request as ready for review September 2, 2022 10:53

pksjce requested a review from a team September 2, 2022 10:53

pksjce temporarily deployed to github-pages September 2, 2022 11:01 Inactive

joshblack reviewed Sep 2, 2022

View reviewed changes

colebemis reviewed Sep 2, 2022

View reviewed changes

pksjce closed this Sep 8, 2022

pksjce reopened this Sep 15, 2022

pksjce added 4 commits September 15, 2022 15:40

Add a new component called Hidden to handle responsive hide/show of c…

05435c4

…ontent

Create four-pandas-roll.md

9590976

Add Hidden component implementation

8af384d

Add as a draft component

02c5642

pksjce force-pushed the pk/hidden-component branch from 9d353c1 to 02c5642 Compare September 15, 2022 05:40

pksjce temporarily deployed to github-pages September 15, 2022 05:46 Inactive

pksjce requested review from colebemis and joshblack September 15, 2022 10:28

joshblack reviewed Sep 16, 2022

View reviewed changes

siddharthkp approved these changes Sep 29, 2022

View reviewed changes

Merge branch 'main' into pk/hidden-component

685fdb8

pksjce enabled auto-merge (squash) October 11, 2022 02:03

pksjce temporarily deployed to github-pages October 11, 2022 02:08 Inactive

pksjce merged commit 45afa3d into main Oct 11, 2022

pksjce deleted the pk/hidden-component branch October 11, 2022 02:10

primer-css mentioned this pull request Oct 11, 2022

Release tracking #2365

Merged

		type="string or Array"
		description="Can be one or more values of 'narrow', 'wide', 'regular'"

-    type="string or Array"
-    description="Can be one or more values of 'narrow', 'wide', 'regular'"
+    type={`
+| 'narrow'
+| 'regular'
+| 'wide'
+| Array<'narrow' | 'regular' | 'wide'>`}
+  description="The viewport range(s) at which children should be hidden"

Hidden component #2301

Hidden component #2301

Uh oh!

Conversation

pksjce commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge checklist

Uh oh!

changeset-bot bot commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siddharthkp commented Sep 5, 2022

Uh oh!

pksjce commented Sep 8, 2022

Uh oh!

pksjce commented Sep 15, 2022

Uh oh!

joshblack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siddharthkp 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pksjce commented Sep 2, 2022 •

edited

Loading

changeset-bot bot commented Sep 2, 2022 •

edited

Loading

github-actions bot commented Sep 2, 2022 •

edited

Loading

siddharthkp left a comment •

edited

Loading