Input, select + other form component updates #32
Conversation
…+ `eye-slash`, `chevron down`, `check` for checkbox; Add `checked-option` svg for Option and Menu
… {component}.styles.ts
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| <sl-tag variant="neutral" size="small" pill> Since Shoelace {{component.since or '?' }} | ||
| </sl-tag> | ||
| <sl-tag size="small" variant="{{ badgeVariant }}" pill style="text-transform: capitalize"> | ||
| {% extends "default.njk" %} {# Find the component based on the `tag` front matter #} {% set component = |
There was a problem hiding this comment.
What's going on with the spacing in this block? It looks like it might have lost some readability here
There was a problem hiding this comment.
This happened because VS code didn't auto-select nunjucks as the language mode & it reformatted the entire page automatically as HTML after I made a small change. Realized I can manually set the language mode to nunjucks to put the formatting back, so this is now fixed on the new rebased branch I just pushed
| {% endfor %} | ||
| </ul> | ||
| {% endif %} {% endblock %} |
There was a problem hiding this comment.
I can't really tell from looking at this diff, but are all of the changes in this file just spacing / indentation stuff? Really wish GH was better at understanding when that's the case, or displaying it intelligently.
There was a problem hiding this comment.
No, the spacing & indentation was just VS code being weird (see my other comment about the nunjucks formatting). The change here was to the "Guidelines" section (line 81) -- I realized the anchor links were not working properly (because of the way the nunjucks file is setup, I think?) for the the Guidelines subheadings, so I just switched them to have one "Usage Guidelines" header with just bolded section titles below.
| @@ -1765,6 +1773,10 @@ sl-card.small-footer::part(footer) { | |||
| margin-bottom: 1rem; | |||
| } | |||
|
|
|||
| sl-breadcrumb.component-header::part(base) { | |||
| @@ -1,6 +1,6 @@ | |||
| # Changelog | |||
|
|
|||
| ## 2.0.1 | |||
| ## 2.0.2 | |||
There was a problem hiding this comment.
I think a PR this size warrants a bump up by minor version, to 2.1.0.
There was a problem hiding this comment.
Ah, this was me fixing my typo from the last patch release (where I repeated 2.0.1 twice). Now moot since we're releasing everything as 2.1.0 with the new upgarde!
| ``` | ||
|
|
||
| Once all that is set up, you'll start your app like normal, i.e. `rails server` and `yarn build:all`. After each change in Shoelace, you'll need to run `npm run build` to generate the production build that `shared-ui` picks up. If you're using `yarn build --watch` and `yarn build:css --watch`, you'll need to restart those after each change as well. | ||
| Once all that is set up, you'll start your app like normal, i.e. `rails server` and `yarn build:all`. After each change in Shoelace, you'll need to run `npm run build` to generate the production build that `design-system` picks up. If you're using `yarn build --watch` and `yarn build:css --watch`, you'll need to restart those after each change as well. |
There was a problem hiding this comment.
The canonical command for starting our apps is now yarn dev, and the watch commands below are no longer necessary. You also don't actually need to re-build on every change; that was a relic of the older version of Shoelace. We can replace this whole paragraph with:
Once all that is set up, you'll start your app like normal via `yarn dev`. Changes made in Shoelace should automatically be reflected in your app.
There was a problem hiding this comment.
okay! updated in the new rebased branch!
| @@ -261,7 +274,7 @@ export default css` | |||
| display: inline-flex; | |||
| align-items: center; | |||
| justify-content: center; | |||
| font-size: inherit; | |||
| font-size: var(--sl-font-size-large); | |||
There was a problem hiding this comment.
This change worries me a bit. Are we completely sure we always want it to be large text?
There was a problem hiding this comment.
This is for classes that are applied to the "clear" and password "eye" icons only -- not for the main input text. With the old system icons, the size could be the same as the font-size of the main input, but with the Font Awesome icons we're using now, they work better at a slightly larger size (20px) than the main input text (16px)
| } | ||
|
|
||
| :host(:focus-visible) .menu-item { | ||
| :host(:focus-visible:not([aria-disabled='true'])) .menu-item { |
| @@ -78,7 +78,7 @@ export default css` | |||
|
|
|||
| .tag--small { | |||
| font-size: var(--sl-button-font-size-medium); | |||
| height: calc(var(--sl-input-height-small) * 0.9); | |||
| height: calc(var(--sl-input-height-small) * 0.85715); | |||
There was a problem hiding this comment.
Is this because Inter is slightly different from their default font?
There was a problem hiding this comment.
This is more because we decided to make our input heights slightly different from the Shoelace default, so if we apply the same * .8 calc to our input heights, we'll get a decimal value height for our tags, which we wanted to avoid.
|
Closing this, as a rebased version of this branch was already merged a while ago |
👉 Review app
Input
label-tooltipandcontext-noteslots and attributescurrencywith default prefix and suffix elementsemail,tel, andsearchpill,filled, sizesmall)overrides.cssinto componentstyles.tsfileTextarea
label-tooltipandcontext-noteslots and attributesfilled, sizessmall,large)overrides.cssinto componentstyles.tsfileSelect
label-tooltipandcontext-noteslots and attributespill,filled, sizesmall)overrides.cssinto componentstyles.tsfileOption
overrides.cssinto componentstyles.tsfileMenu, Menu Item, Menu Label
overrides.cssinto componentstyles.tsfileMisc
eye,eye-slash(used inpasswordInput)x-circle-fill(used inclearableInput)chevron-down(used in Select)checked(used in Checkbox)checked-optionfor use in Option and Menu to show selected options and menu items