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(keyboard-key): add documentation #4164

Open
wants to merge 82 commits into
base: main
Choose a base branch
from

Conversation

krisantrobus
Copy link
Collaborator

@krisantrobus krisantrobus commented Nov 19, 2024

@jhanvi1399
Copy link

  • suggestion: about keyboard key can be changed to h3 instead of h2.

  • question: in the about keyboard key since we have the blockquote design, is it possible for us to use that or should we wait until its release?

  • suggestion: keyboard key tooltips and hooks section can be h2, useKeyCombination and useKeyCombinations section can be h3 under hooks.

  • suggestion: common keyboard shortcuts and when to use keyboard section key can be h2.

  • nitpick: i feel lists on the page have some extra margin from the left. very nitpicky no stress.

All else looks great!!!

@krisantrobus
Copy link
Collaborator Author

  • suggestion: about keyboard key can be changed to h3 instead of h2.
  • suggestion: common keyboard shortcuts and when to use keyboard section key can be h2.

Good catch, fixed in latest commit

  • suggestion: keyboard key tooltips and hooks section can be h2, useKeyCombination and useKeyCombinations section can be h3 under hooks.

I originally kept these under examples but the spec doc is structured differently so I will make these changes

  • question: in the about keyboard key since we have the blockquote design, is it possible for us to use that or should we wait until its release?

It would depend on whether the blockquote will be merged before this docs. We do have other blockqoutes throughout the site so we should do a cleanup as part of the docs ticket for Blockquote.

  • nitpick: i feel lists on the page have some extra margin from the left. very nitpicky no stress.

I double checked and I'm not adding any extra margins left, only bottom for the List component

@jhanvi1399
Copy link

All looks good!
I just crossed check the list in keyboard spec vs the list component. I think in the list component, the bullet points start inside the margin itself and in keyboard they start after the margin. Maybe that is why it looks a lil more indented.

  • Keyboard Spec Page
image
  • List Component
image

@krisantrobus
Copy link
Collaborator Author

Very good point @jhanvi1399! I looked through and saw that list was using the markdown syntac but I'm using the list component which renders it in a different way. Pushed up a fix in latest commit.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 26, 2024
@krisantrobus krisantrobus added 🚀 merge it! 🕵🏻‍♀️ Run website visual regression When applied, we will run a full suite of visual regression tests across the doc site labels Nov 26, 2024
Copy link

cypress bot commented Nov 26, 2024

Paste    Run #9100

Run Properties:  status check passed Passed #9100  •  git commit e9e663ae0b ℹ️: Merge 4bed0a903dbc680928aca55e292ab8e9981331b4 into 7f39856ee3697235b535ea02eaef...
Project Paste
Branch Review docs/keyboard-key-init
Run status status check passed Passed #9100
Run duration 06m 09s
Commit git commit e9e663ae0b ℹ️: Merge 4bed0a903dbc680928aca55e292ab8e9981331b4 into 7f39856ee3697235b535ea02eaef...
Committer krisantrobus
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 125
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Components Related to the component library (core) of this system Area: Doc Site Related to the documentation website Area: Repo lgtm This PR has been approved by a maintainer 🚀 merge it! 🕵🏻‍♀️ Run website visual regression When applied, we will run a full suite of visual regression tests across the doc site size:XL This PR changes 500-999 lines, ignoring generated files. Type: Documentation Improvements or additions to documentation Type: Tests Adds tests to the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants