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

[Docs] Consolidated GuidePage and GuideTabbedPage into just GuideTabbedPage #5887

Merged
merged 6 commits into from
May 10, 2022

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented May 8, 2022

Now GuideTabbedPage can do everything GuidePage could on top of supplying pages. This also now removes the page-tabbed version of Playground (all of them have been converted to the example tab). There was really no reason why they needed to be separate and trying to keep them in sync was too manual.

Everything should behave just as it did, just with one less layout to maintain.

The GuideSection has also been updated to normalize padding when using color.

  • color=undefined: This is our default basic example setup with no changes except little extra was added between examples
  • color=defined: Meaning, any value is manually passed it will get added vertical spacing.

But only when in the larger breakpoints. Otherwise, all sections are white (because they're so large) and smaller areas like ThemeExamples will get shaded.

Screen Shot 2022-05-08 at 17 58 06 PM

Screen Shot 2022-05-08 at 17 57 39 PM

Checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • [ ] Props have proper autodocs and playground toggles
  • [ ] Added documentation
  • [ ] Checked Code Sandbox works for any docs examples
  • [ ] Added or updated jest and cypress tests
  • [ ] Checked for breaking changes and labeled appropriately
  • [ ] Checked for accessibility including keyboard-only and screenreader modes
  • [ ] Updated the Figma library counterpart
  • [ ] A changelog entry exists and is marked appropriately

@cchaos cchaos added documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog labels May 8, 2022
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5887/

@elizabetdev elizabetdev self-requested a review May 9, 2022 12:33
Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

Tested in Chrome, Firefox, Safari, and Edge.

Comment on lines +55 to +61
<GuideTabbedPage
title="Sizing"
isBeta={!showSass}
notice={<ThemeNotice />}
showThemeLanguageToggle
>
<GuideSection>
<GuideSection color="transparent">
Copy link
Contributor

Choose a reason for hiding this comment

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

The first <GuideSection /> in sizing page doesn't start with color="subdued" like in the other pages: typography, colors and borders. This process to change the color seems very manual.
So should we have a prop in <GuideTabbedPage /> called alternateSectionsColors (or a better naming) and if the prop is true all <GuideSection /> will get the color alternated starting with subdued then transparent.

Something like:

// in src/components/guide_tabbed_page/guide_tabbed_page.tsx
<EuiPageContent
  css={css`
    ${alternateSectionsColors &&`
    > .euiPanel:nth-child(even) {
        background-color: transparent;
    }
    > .euiPanel:nth-child(odd) {
        background-color: ${panelSubduedColor};
    }
  `}
`}
/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it's meant to be pretty manual because it's intentionally not trying to be zebra striping. It's supposed to only shade sections with ThemeExamples and not theme values (see in screenshot above). If we try to just do it automatically like you have it will likely shift which section will get shaded if there aren't value sections like on page: https://eui.elastic.co/pr_5887/#/theming/sizing. I think it's better to default to plain transparent/white background, making us more mindful of when we shade the backgrounds. Otherwise, I can see us fighting against the auto-child shading.

@cchaos
Copy link
Contributor Author

cchaos commented May 10, 2022

If no one has any objections to merging these docs pages, I"ll go ahead and merge the PR in an hour.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5887/

@cchaos cchaos merged commit 5a75a75 into elastic:main May 10, 2022
@cchaos cchaos deleted the docs/one_tabbed_page branch May 10, 2022 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants