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

chore(docs): added mobile jumplinks and example styling #2867

Merged
merged 10 commits into from
Mar 26, 2022

Conversation

evwilkin
Copy link
Member

@evwilkin evwilkin commented Mar 8, 2022

Closes #2843

This PR:

  • Unhides the jumplinks which are currently hidden at <=1450px and utilizes the component's mobile layout plus custom styles from designs in Improvements to Component pages #2843
  • Removes background from example headers/controls
  • Remove borders from CodeEditorControls below each example, except for the blue bottom border on hover/focus/active
  • Remove border below .pf-c-code-editor__header above example code except when example code is expanded
  • Simplifies copy function for example code to fix console error currently being thrown

@patternfly-build
Copy link
Contributor

patternfly-build commented Mar 8, 2022

@evwilkin
Copy link
Member Author

evwilkin commented Mar 8, 2022

@mcarrano @maryshak1996 a couple notes/questions on this PR:

  1. The jumplinks component has a mobile layout (demo) that seems to match the design, including being expandable without the need to remove an extra blue left border, so I've simply used the mobile layout of jumplinks with custom styling added rather than wrap it in the expandable disclosure.
  2. Our examples are inconsistent when it comes to the modifiers - for example, the card modifiers example includes these toggles above the example, while the card with image and actions example includes a toggle after the example and this tooltip example has a completely different look altogether.
    • Based on this, I'd suggest splitting this out into a bigger consideration for a common design/implementation to be used for these types of examples.
    • If we go this route, should we proceed with the example header/code editor controls having the grey background as seen, and leave the modifiers untouched for a follow-up issue?

@evwilkin
Copy link
Member Author

@mcarrano @maryshak1996 I've updated this PR per our discussion:

  • Added bottom box shadow to mobile table of contents (large when collapsed, small when expanded)
  • Examples header & code editor controls no longer have any background, match page color instead
  • Code editor controls are now plain icon buttons with no borders/background

@mcarrano
Copy link
Member

@evwilkin thanks for making these updates. Overall I think this looks a whole lot better! I just have one question/nit. By making the tool buttons below the example plain buttons, they no longer display a selected state. You'll notice that in the existing site, when the code window is open the "<>JS" or "<>TS" button is shown as selected/active. I'm not sure that's a big deal, but I wanted to raise it as an issue. What do you think @maryshak1996 ?

In looking at this, I was also wondering how those are implemented now. Even though they function as buttons, they have dropdown styling that leads to them having a blue underline when open. Were these some type of custom button? Would we want to customize the plain button to give it a selected state?

@evwilkin
Copy link
Member Author

@mcarrano thanks for taking a look. The controls beneath each example & the expandable code below them are the CodeEditor component with custom controls.

The CodeEditorControls output control variant buttons (<Button variant="control">), which apply the styling currently on our site. My understanding was to remove all the borders on these buttons (which in turn removed the blue bottom border), but we could keep only that blue border if we'd like - I believe this would be a custom style for our site.

@mcarrano
Copy link
Member

@evwilkin thanks for the added info. Yes I do recall that we styled the code editor buttons that way. @maryshak1996 I'll defer to you, should we do any more customization of these buttons or do you think they are good as is?

@maryshak1996
Copy link

@evwilkin @mcarrano if we could keep just the blue border to indicate active state, that would be ideal!

…ls, updated example background colors to variables
@evwilkin
Copy link
Member Author

@mcarrano @maryshak1996 per our conversation I've made 2 additional changes:

  • removed the borders from the example CodeEditorControls but kept the blue bottom border on focus/hover/active
  • Remove the bottom border beneath the examples' CodeEditorControls when the example code is collapsed.

Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

This looks great @evwilkin . Thanks for making these updates!

@evwilkin evwilkin merged commit c7b9705 into patternfly:main Mar 26, 2022
jessiehuff pushed a commit to jessiehuff/patternfly-org that referenced this pull request Oct 24, 2022
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.

Improvements to Component pages
4 participants