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

[feat]: <rh-code-block> #328

Closed
eohammond opened this issue May 25, 2022 · 30 comments · Fixed by #995
Closed

[feat]: <rh-code-block> #328

eohammond opened this issue May 25, 2022 · 30 comments · Fixed by #995
Assignees
Labels
for dev Ready for development priority: high High priority

Comments

@eohammond
Copy link
Collaborator

eohammond commented May 25, 2022

<rh-code-block>

Mockups

Options/Feature priorities

Taken from Benny's comment below

  1. base style
  2. color context
  3. copy button (blocked on copy button)
  4. resizable
  5. expandable
  6. line wrap (blocked on switch)
  7. syntax (blocked on code tokens)

Patterns

To be implemented later

  1. toolbar with (switch|copy|links)
  2. tabs with (switch|copy)
@eohammond eohammond added the for dev Ready for development label May 25, 2022
@zeroedin
Copy link
Collaborator

zeroedin commented May 25, 2022

We would only remove pfe-codeblock if it did not exist in Patternfly proper. Because there is a Patternfly version this would be a 1:1 migration. pfe-codeblock would then become a 1:1 match with its counterpart in Patternfly, then the rh-codeblock component would extend the pfe version by applying our design system customizations. An example of this is @bennypowers work on pfe-modal -> rh-dialog

pfe-modal patternfly/patternfly-elements#2036
rh-dialog #288

@eohammond eohammond changed the title [feat]: <tag> ready to migrate [feat]: <rh-codeblock> ready to migrate May 25, 2022
@coreyvickery
Copy link
Collaborator

coreyvickery commented May 25, 2022

Requirements

  • Light and dark themes
  • Accessible color styles within both themes
  • Copy to clipboard button
  • Expandable Show more button or function

Current design to work from

https://xd.adobe.com/view/a64d7305-b6cb-439c-8caa-1039834fa1b8-f31e/

Relevant chats

  • We also have copy to clipboard as a separate component (MC)
  • I think we could implement a cleaner version of the expandable section (MC)
  • It feels like the "show more" should be in a button maybe, so it doesn't feel awkward or lost next to the code (MC)
  • Actually, "copy" is part of the PFE implementation too—on hover (MC)

@methomps methomps added the for design Design work is requested label May 25, 2022
@methomps
Copy link
Collaborator

Cloud.redhat.com's use of codeblock is not reliant on our implementation. AKA we are not a blocker.

@methomps methomps removed the for dev Ready for development label May 25, 2022
@coreyvickery
Copy link
Collaborator

@methomps Possible to provide any screenshots from that environment? I'm having trouble logging into my account.

@methomps
Copy link
Collaborator

methomps commented Jun 7, 2022

Oh boy, I'll need @dcaryll and/or @eohammond to assist with this. I don't know much about this issue at all. Dustin may be able to assist too.

@eohammond
Copy link
Collaborator Author

@coreyvickery The request was to use the codeblock pattern on cloud.redhat.com. I believe Dustin had a use case for codeblock on learning paths, like this page for instance Do you need screenshots from console.redhat.com?

@coreyvickery
Copy link
Collaborator

@eohammond If you're able to provide, yes.

@coreyvickery
Copy link
Collaborator

coreyvickery commented Jun 7, 2022

@methomps This is being reviewed by leads from other teams.

https://xd.adobe.com/view/a64d7305-b6cb-439c-8caa-1039834fa1b8-f31e/

@methomps methomps moved this from Backlog to Review in Red Hat Design System Jun 7, 2022
@coreyvickery coreyvickery moved this from Review to Backlog in Red Hat Design System Jun 16, 2022
@coreyvickery
Copy link
Collaborator

@methomps We are still blocked on this until we sync with the other design system leads to see what research and/or design they have already done, will try to move this forward early next week.

@coreyvickery coreyvickery changed the title [feat]: <rh-codeblock> ready to migrate [feat]: <rh-code-block> Jun 16, 2022
@methomps methomps added the blocked Issue is blocked label Jun 17, 2022
@methomps
Copy link
Collaborator

Probably a good candidate to test our new design process once we've defined that process a bit more with the leads collaborative.

@bennypowers
Copy link
Member

@bennypowers
Copy link
Member

Ideally we wouldn't have to create this at all, and rather use a prism or highlight.js syntax theme. Talking it over with @mwcz, we agreed that doing code highlighting client-side is not ideal. This could be useful for certain cases like cms or places where it's cumbersome to server-render code, but shouldn't be the first choice.

On the implementation side, if we do implement this (and pfe-codeblock), I'd prefer to either fork or white-label the existing <zero-md> element.

cc @markcaron @heyMP

@bennypowers bennypowers added for dev Ready for development and removed for design Design work is requested labels May 18, 2023
@bennypowers bennypowers moved this from Review to In Progress in Red Hat Design System May 18, 2023
@bennypowers bennypowers moved this from In Progress to Backlog in Red Hat Design System May 18, 2023
@coreyvickery
Copy link
Collaborator

coreyvickery commented May 18, 2023

Current rh-code-block design:

https://xd.adobe.com/view/a64d7305-b6cb-439c-8caa-1039834fa1b8-f31e/

@coreyvickery
Copy link
Collaborator

@bennypowers
Copy link
Member

bennypowers commented May 18, 2023

priorities:

  1. base style
  2. color context
  3. copy button (blocked on copy button)
  4. resizable
  5. expandable
  6. line wrap (blocked on switch)
  7. syntax (blocked on code tokens)

patterns

  • toolbar with (switch|copy|links)
  • tabs with (switch|copy)

@markcaron markcaron moved this from Needs Design Review to Done in DPO: Design Leads Review May 18, 2023
@markcaron
Copy link
Collaborator

For line highlighting, see <mark> element in MDN Web Docs

@bennypowers
Copy link
Member

@coreyvickery grey background and border colours #F5F5F5 and #D2D2D2 aren't token values, please advise

@coreyvickery
Copy link
Collaborator

@bennypowers I forgot the token names because I’m mobile, but F5 -> F2 and D2 -> C7. Those hex values should map to tokens.

@bennypowers
Copy link
Member

Thanks I'll check tomorrow and get back to you if I have more questions 🙏

@bennypowers
Copy link
Member

can you elaborate on the resize behaviour?
What happens before, during, and after the resize action (i.e. drag handle)
it looks in the mockup like prior to resizing the single long line of text is broken into two, but arbitrarily, while there's also a horizontal scrollbar, whereas after resize, the text wraps at the element width. is that correct? is there a prescribed width for the scrollable container before resize?

@coreyvickery
Copy link
Collaborator

@bennypowers I apologize for the XD not being 100% clear.

  • There is fixed width guidance on screen 2, I thought 1000px would be good, but that might be too wide after all, maybe I or we can choose another smaller value
  • I was thinking the resize could be vertical only, is this doable or advisable?
  • I think what you are saying is correct, it might be easier or faster to put together a super rough demo

Ping me right away if you need me to mock up anything more.

@bennypowers
Copy link
Member

yes resize can be in either both or none directions: https://developer.mozilla.org/en-US/docs/Web/CSS/resize

I'd like to see a rough ux demo of the intention here. it's not clear to me yet how much of this we can accomplish easily with css

@bennypowers
Copy link
Member

perhaps we can workshop this for a bit at office hours

@coreyvickery
Copy link
Collaborator

@bennypowers I will not be able to make it, but please do so and let me know if we need to change the design or rethink this feature altogether.

@bennypowers
Copy link
Member

could you export svgs for the action icons please?

@bennypowers
Copy link
Member

I did a little work on copy buttons for code block (only) this morning, based on the xd here, but i'm going to revert that work pending discussion in #423

tl;dr: we can't use tooltip or button text for copy state, should use alert, plus perhaps role=status cc @nikkimk for comments and corrections, with thanks.

@bennypowers
Copy link
Member

bennypowers commented May 22, 2023

Buttons

The button styles in the XD are mostly bespoke (particularly the "action" buttons - the ones superimposed in the top right of the code block). do these represent new button variants, or should I continue to just hack them into bespoke variants private to code block?

Panel/Card/Toolbar

The upper toolbar, is this a component? Are there or will there be other components that fit into this "toolbar with content" deal?

@bennypowers
Copy link
Member

Callouts

can they go in a 'gutter' on the left (inline start) instead of at the end? this would be easier to implement and read (imo) for word-wrapped snippets

@bennypowers bennypowers linked a pull request May 30, 2023 that will close this issue
@bennypowers
Copy link
Member

Today at Office hours we took the decision to release a very basic version of code-block with bellsprout.

features we'll push off to bulbasaur:

  • new variants of <rh-button>: icon button / FAB for copy
  • syntax highlighting
  • callout badges
  • toolbar/container

We also have some usability questions we'd like to address:

  • should copy button always be a FAB (i.e. floating over the codeblock, not slotted into the toolbar)
  • is combining tabs with links/actions a usability problem for sighted keyboard users?

SO, basically the bellsprout version will be a gray box that formats a script of text

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for dev Ready for development priority: high High priority
Projects
Status: Done ☑️
Status: Done ☑️
Development

Successfully merging a pull request may close this issue.

8 participants