-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Button Playbook #5
base: next
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than naming and a formatting tweak the rough plan looks good to me! Will defer to Daross on checking the functionality of the proposed replacements. :)
@@ -2,4 +2,5 @@ | |||
|
|||
- ## [Protips](/teamshares/recipes/protips.md) | |||
- ## [Rails UJS](/teamshares/recipes/rails-ujs.md) | |||
- ## [Converting Button View Component To Shoelace](/teamshares/recipes/buttons.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kdonovan updated naming here as well, do you think it's worth to change the file name? Or keep since this is the only recipe book for buttons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's just tweak it to e.g. shared-ui-buttons.md
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this, I'm realizing that we should probably:
a. Make the link titles short and pithy so they fit easily in the sidebar, and
b. Add a little description line under each of them on this recipes.md
page
In this case, I'd make the title just Converting SharedUI Buttons
, and then under that a line that says Recipes for replacing various SharedUI::ButtonComponent usages with sl-button
, to make it super explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks alright to me, once @CrookedGrin signs off on the contents of the replacements (and... maybe this src/components/tab-group/tab-group.test.ts
test is failing in webkit?)
@@ -2,4 +2,5 @@ | |||
|
|||
- ## [Protips](/teamshares/recipes/protips.md) | |||
- ## [Rails UJS](/teamshares/recipes/rails-ujs.md) | |||
- ## [Converting Button View Component To Shoelace](/teamshares/recipes/buttons.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's just tweak it to e.g. shared-ui-buttons.md
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great content! A bunch of minor formatting requests and some additional explanatory context, and this will be good to go.
@@ -2,4 +2,5 @@ | |||
|
|||
- ## [Protips](/teamshares/recipes/protips.md) | |||
- ## [Rails UJS](/teamshares/recipes/rails-ujs.md) | |||
- ## [Converting Button View Component To Shoelace](/teamshares/recipes/buttons.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this, I'm realizing that we should probably:
a. Make the link titles short and pithy so they fit easily in the sidebar, and
b. Add a little description line under each of them on this recipes.md
page
In this case, I'd make the title just Converting SharedUI Buttons
, and then under that a line that says Recipes for replacing various SharedUI::ButtonComponent usages with sl-button
, to make it super explicit.
|
||
## 1. Link button | ||
|
||
> 💡 Hint: these are buttons marked with `external: true` or marked with `render_as_link: true` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like these little blockquote hints. The italicized emoji looks a bit weird, but we can figure out formatting for that later.
I think some of the hints here should just be paragraphs of explanation, in which I'd err on the side of over-explaining. Here, I would just make this a paragraph that says "SharedUI::ButtonComponent
s with external: true
or render_as_link: true
are rendered as HTML <a>
tags. To get a similar effect in Shoelace, use sl-button
with an href
attribute."
| Schedule prep session | ||
``` | ||
|
||
> 💡 Hint: use `target="_blank"` to open the link in a new tab |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then short little tips like this can remain as "hints".
|
||
## 2. Submit button | ||
|
||
> 💡 Hint: these are commonly used with simpleform and have `submit: true` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here again I would lengthen this explanation a bit and make it just a regular paragraph
|
||
### Before | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all of these code blocks, you can get the right formatting via
```pug
|
||
### After (this example uses a content tag) | ||
|
||
```jsx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pug
and multiple-lines here as well
``` | ||
|
||
## 5. Modal button | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the modals in Buyout are a bit different from the ones in OS, so we may need to update this to make that clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would also add a little context about what we're talking about here, i.e., "To replace SharedUI::ButtonComponent
s that are used to launch modals..."
|
||
> 💡 For more information check out [our Rails UJS recipe page](https://design.teamshares.com/#/teamshares/recipes/rails-ujs) | ||
|
||
```jsx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pug
|
||
## 6. Internal link/path with no explicit associated form | ||
|
||
> 💡 Hint: for :get requests you can just use a link button above, for :delete, :post, :patch, etc. we now have a ButtonTo view component that behaves as rails’ [button_to](https://apidock.com/rails/ActionView/Helpers/UrlHelper/button_to) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to put the :get
, :post
, ButtonTo
etc in code format for readability
|
||
### After | ||
|
||
```jsx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!! This looks great. I noticed in the Recipes side nav section, the link to this playbook doesn't route to the correct page, it only works from the actual recipes page. Heres a video demoing the issue:
Import the following playbook to our design system docs:
https://www.notion.so/teamshares/Button-Playbook-3341236385ca44ccac0eb81deb72720b?pvs=4
Preview it by clicking "Visit Preview" below and then "Recipes" > "Buttons"