Input, select + other form component updates [Rebased] #34

slhinyc · 2024-03-13T17:20:40Z

👉 Review app

Rebased version of this approved PR
Added a prettier formatting commit here
Plus a small text update to contributing.md based on comment from other PR
Added all the changes from the PR to changelog.md

Changes in this PR

Input

Add label-tooltip and context-note slots and attributes
Add new type currency with default prefix and suffix elements
Add default prefix icon options to types email, tel, and search
Update documentation examples to call out or hide unused patterns (pill, filled, size small)
Add icon & usage guidelines
Moved styles from overrides.css into component styles.ts file
Minor styling updates

Textarea

Add label-tooltip and context-note slots and attributes
Update documentation examples to call out or hide unused patterns (filled, sizes small, large)
Moved styles from overrides.css into component styles.ts file
Minor styling updates

Select

Add label-tooltip and context-note slots and attributes
Update documentation examples to call out or hide unused patterns (pill, filled, size small)
Add icon & usage guidelines
Moved styles from overrides.css into component styles.ts file
Minor styling updates

Option

Update documentation examples
Add icon guidelines
Moved styles from overrides.css into component styles.ts file
Minor styling updates

Menu, Menu Item, Menu Label

Update documentation examples
Add icon guidelines
Moved styles from overrides.css into component styles.ts file
Minor styling updates

Misc

Bump Font Awesome registry to v6.5.0
Replace system icons with Font Awesome svgs
- eye, eye-slash (used in password Input)
- x-circle-fill (used in clearable Input)
- chevron-down (used in Select)
- checked (used in Checkbox)
Add new system icon checked-option for use in Option and Menu to show selected options and menu items

…ting updates

…ines

…delines

…lines

…+ `eye-slash`, `chevron down`, `check` for checkbox; Add `checked-option` svg for Option and Menu

… {component}.styles.ts

…hares app

vercel · 2024-03-13T17:20:47Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
shoelace	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 14, 2024 3:42pm

docs/_includes/component.njk

CrookedGrin · 2024-03-13T21:32:21Z

src/components/menu-item/menu-item.styles.ts

@@ -20,8 +20,8 @@ export default css`
    font-weight: var(--sl-font-weight-normal);
    line-height: var(--sl-line-height-normal);
    letter-spacing: var(--sl-letter-spacing-normal);
-    color: var(--sl-color-neutral-700);
-    padding: var(--sl-spacing-2x-small) var(--sl-spacing-2x-small);
+    color: var(--ts-color-text-default);


I have reservations about using the ts-- variables directly in the Shoelace stylesheets, because then we're kind of creating a circular dependency. As I understand it, we shouldn't need to use those values directly, because we're overriding the Shoelace values with their corresponding ts values already. Open to discussion on that.

Ah, I see -- so we should use --sl-color-neutral-900 here, you mean? I was thinking this makes the code more readable for us, since we know right away what ts-color-text-default is. But I'm fine to update all these to remove the --ts variable and replace with the --sl equivalent.

There are a few places where we had to create our own hard-coded --ts token rather than overriding an existing --sl token because the value we wanted kinda fell between two --sl tokens that we wanted to keep. In those cases, do you think it's okay to reference the --ts token directly?

Also -- okay with you if I just keep that in mind as the new approach & update these instances in the new branch that I'm working on?

Yeah, that's fine to just update in the other branch. I think if we're creating new values, we might want to just create those as --sl values as well, so we have consistency in our approach across the board.

ah, okay. we have several of these custom -ts values so i think i will pull this work of updating this into a separate ticket, then? but what you are saying makes sense to me. agree that we should be consistent!

Yep, that's fine.

CrookedGrin · 2024-03-13T21:44:43Z

src/components/tag/tag.styles.ts

@@ -75,7 +75,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);


We might want to make sure these render OK in different browsers / platforms. I feel like some of them are pickier about how they render odd font sizes.

Ah, great call. What do you think if I just did away with the calc function entirely and just add a variable for the height that we know we want these to be? Feels more straightforward? If that sounds good to you, I'll update.

I just pushed a commit to update the * by decimal to a - by a rem value that gives us the same effective height that we want

Sounds good, although I'm not seeing that reflected here yet

See my comment below

src/styles/exports/overrides.css

src/styles/form-control.styles.ts

CrookedGrin

Overall looks great, just a few comments/questions.

…ecimal

slhinyc · 2024-03-14T19:01:47Z

src/components/tag/tag.styles.ts

@@ -75,21 +75,21 @@ export default css`

  .tag--small {
    font-size: var(--sl-button-font-size-medium);
-    height: calc(var(--sl-input-height-small) * 0.85715);
+    height: calc(var(--sl-input-height-small) - 0.25rem); /* 24px */


@CrookedGrin -- Here is that change from the * decimal to - a rem value! I don't know why github doesn't update the code block near the comment or at least give us the option of seeing the "old version" and the "new version"? 🤷‍♀️

Hmm, weird. Anyway, looks good!

slhinyc and others added 18 commits March 13, 2024 12:05

Update calc value for small and large tags to force whole number

283bbce

Add "Form Controls" page back to sidebar list; minor docs page format…

0723410

…ting updates

Update Input component properties, styles, doc site examples & guidel…

925c0ae

…ines

Update Textarea component properties, styles, doc site examples & gui…

b15a7b6

…delines

Update Select component properties, styles, doc site examples & guide…

5b05694

…lines

Update Option component styles, doc site examples, and guidelines

f7f8476

Update Menu component styles, doc site examples, guidelines

c4ead5d

Bump Font Awesome library to latest 6.5.1

0c8befe

Replace system icon svgs to Font Awesome set: x-circle-fill, eye …

38bc443

…+ `eye-slash`, `chevron down`, `check` for checkbox; Add `checked-option` svg for Option and Menu

Move component overrides styles from overrides.scss and directly into…

aa09934

… {component}.styles.ts

Update Figma component status for Dialog and Dropdown

0416743

Add note that context note will wrap below the label

55dbeb0

Add Font Awesome version note

e6a0fac

Fix guidelines overflow & anchor link issue; fix changelog typo

69d3378

Update local testing directions on contributing page

e1da5b4

Minor spacing, text-color updates after testing in cash + OS

dc36a68

Formatting update from prettier

eaf98a9

Update contributing.md instructions for connecting to a local Teams…

4eeff45

…hares app

slhinyc requested a review from a team March 13, 2024 17:20

slhinyc requested a review from CrookedGrin as a code owner March 13, 2024 17:20

Add changelog notes for changes in this PR

095c3fe

vercel bot deployed to Preview March 13, 2024 21:19 View deployment

CrookedGrin reviewed Mar 13, 2024

View reviewed changes

docs/_includes/component.njk Outdated Show resolved Hide resolved

CrookedGrin reviewed Mar 13, 2024

View reviewed changes

src/styles/exports/overrides.css Show resolved Hide resolved

CrookedGrin reviewed Mar 13, 2024

View reviewed changes

src/styles/form-control.styles.ts Show resolved Hide resolved

CrookedGrin approved these changes Mar 13, 2024

View reviewed changes

Another try for fixing the nunjucks formatting

d83a680

vercel bot deployed to Preview March 13, 2024 23:08 View deployment

Update calc for tag heights to subtract rather than multiply by odd d…

86af1f4

…ecimal

vercel bot deployed to Preview March 14, 2024 15:42 View deployment

slhinyc commented Mar 14, 2024

View reviewed changes

CrookedGrin merged commit 6074cfc into next Mar 14, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Input, select + other form component updates [Rebased] #34

Input, select + other form component updates [Rebased] #34

slhinyc commented Mar 13, 2024 •

edited

Loading

vercel bot commented Mar 13, 2024 •

edited

Loading

CrookedGrin Mar 13, 2024

slhinyc Mar 13, 2024

CrookedGrin Mar 14, 2024

slhinyc Mar 14, 2024

CrookedGrin Mar 14, 2024

CrookedGrin Mar 13, 2024

slhinyc Mar 13, 2024

slhinyc Mar 14, 2024

CrookedGrin Mar 14, 2024

slhinyc Mar 14, 2024

CrookedGrin left a comment

slhinyc Mar 14, 2024

CrookedGrin Mar 14, 2024

Input, select + other form component updates [Rebased] #34

Input, select + other form component updates [Rebased] #34

Conversation

slhinyc commented Mar 13, 2024 • edited Loading

Changes in this PR

Input

Textarea

Select

Option

Menu, Menu Item, Menu Label

Misc

vercel bot commented Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CrookedGrin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slhinyc commented Mar 13, 2024 •

edited

Loading

vercel bot commented Mar 13, 2024 •

edited

Loading