Skip to content

Add Dropdown "quiet" variant #96

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 8, 2024
Merged

Add Dropdown "quiet" variant #96

merged 1 commit into from
May 8, 2024

Conversation

clintcs
Copy link
Collaborator

@clintcs clintcs commented May 7, 2024 •
edited
Loading

🚀 Description

Adds variant?: "quiet" and hide-label: boolean arguments to Dropdown.

📋 Checklist

  • I have read and followed the Contributing Guidelines.
  • I have added tests to cover new or updated functionality.
  • I have created or updated stories in Storybook to document the new functionality.
  • I have included a changeset with this Pull Request if it adds/updates/removes functionality for consumers.
  • I have scheduled a Design Review for these changes, if one is required.
  • I have followed the ARIA Authoring Practices Guide and/or met with the Accessibility Team to ensure this functionality is accessible.
  • I have added the component to exports in packages/components/package.json (if applicable).

🔬 How to Test

A quick comparison with the mockups should do.

📸 Images/Videos of Functionality

image

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 7, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 6be0e50

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@crowdstrike/glide-core-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@clintcs clintcs force-pushed the add-dropdown-quiet-variant branch from 0673ba7 to f8f88ce Compare May 7, 2024 15:17
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 7, 2024

Preview URL

https://glide-core.crowdstrike-ux.workers.dev/add-dropdown-quiet-variant

@clintcs clintcs force-pushed the add-dropdown-quiet-variant branch 2 times, most recently from 5f0fa2a to ea41ee1 Compare May 7, 2024 17:59
clintcs
clintcs commented May 7, 2024
View reviewed changes
packages/components/src/button.stories.ts
@@ -9,7 +9,7 @@ const meta: Meta = {
parameters: {
docs: {
description: {
component: 'A button element with optional slots for icons.',
Copy link
Collaborator Author

@clintcs clintcs May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that Button uses a <button> is an implementation detail 🙂.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be fine deleting a lot of these descriptions in our stories. I think the components pretty much speak for themselves in most cases via the demo. Maybe having notes for our form components about FormData and what not is still helpful, but other than that, I'm not sure I find a ton of value in these. Same for you or no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find much value in them either, honestly. I'll go ahead and merge this as is if that's okay. Then one of us can follow up with a PR that removes the superfluous ones.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a plan to me!

@clintcs clintcs force-pushed the add-dropdown-quiet-variant branch 4 times, most recently from b62142d to f8f6428 Compare May 8, 2024 17:18
@clintcs clintcs marked this pull request as ready for review May 8, 2024 17:18
ynotdraw
ynotdraw approved these changes May 8, 2024
View reviewed changes
packages/components/src/button.stories.ts
@@ -9,7 +9,7 @@ const meta: Meta = {
parameters: {
docs: {
description: {
component: 'A button element with optional slots for icons.',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be fine deleting a lot of these descriptions in our stories. I think the components pretty much speak for themselves in most cases via the demo. Maybe having notes for our form components about FormData and what not is still helpful, but other than that, I'm not sure I find a ton of value in these. Same for you or no?

clintcs
clintcs commented May 8, 2024
View reviewed changes
packages/components/src/dropdown.stories.ts
@@ -143,6 +143,13 @@ const meta: Meta = {
},
type: { name: 'function' },
},
variant: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ynotdraw, @danwenzel: We talked a bit about this in the ticket. I mulled it over. Select does feel like the natural choice for cases like this, though it does throw a wrench in the gears of that ticket.

packages/components/src/button.stories.ts
@@ -9,7 +9,7 @@ const meta: Meta = {
parameters: {
docs: {
description: {
component: 'A button element with optional slots for icons.',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find much value in them either, honestly. I'll go ahead and merge this as is if that's okay. Then one of us can follow up with a PR that removes the superfluous ones.

clintcs
clintcs commented May 8, 2024
View reviewed changes
packages/components/src/dropdown.styles.ts Outdated
font-size: var(--cs-heading-xxxs-font-size);
font-style: var(--cs-heading-xxxs-font-style);
font-weight: var(--cs-heading-xxxs-font-weight);
gap: var(--cs-spacing-xs);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed an amended commit changing this to --cs-spacing-xxs after talking to Mary.

@clintcs clintcs force-pushed the add-dropdown-quiet-variant branch from f8f6428 to 6be0e50 Compare May 8, 2024 20:17
@clintcs clintcs merged commit 6835819 into main May 8, 2024
7 checks passed
@clintcs clintcs deleted the add-dropdown-quiet-variant branch May 8, 2024 20:22
@github-actions github-actions bot mentioned this pull request May 8, 2024
Merged
dylankcrwd added a commit that referenced this pull request May 9, 2024
@dylankcrwd
update modal
d7807f8 
Upgrade Storybook (#84)

Add Toggle (#78)

Add lit-plugin extension (#86)

update modal stories

update story and modal icon button a11y

update story types

Add input (#83)

* Copy component over from previous repo and auto-fix

* Update per new conventions. 100% test coverage

Fix up class names per stylelint

Fix up import paths

Fix up icons

Remove clickOnElement helper

Sufficient to use native element's .click() method

Allow passing of invalid types to input

This matches native: The user can pass whatever
string they want. TypeScript will properly
message to the user if they use an invalid type.

Hit 100% test coverage. Remove unnecessary type narrowing

Remove unused state

Close shadow root for Input, except in tests

Add changeset for Input

Prefer prefixing event handlers with "on"

Fix up icons and icon spacing

Restrict exports (#87)

Switch Tree Item to dot notation (#89)

Checkbox a11y fixes (#90)

* Hide asterisk from screenreader announcement
* Add aria-invalid attr and tests

feat(Tag): move from glide to glide-core (#81)

Adds tag

Add docs + update usages targeting root element with CSS (#85)

Remove label attribute from cs-menu (#92)

Was not being used.
Probably a leftover from a previous iteration in the old repo.

Add a lint rule enforcing private fields and "ElementRef" suffixes for Lit refs (#95)

style(ExampleIcon, Tag): adds "size" variable (#94)

Adds a "size" css variable to `cs-example-icon` and is applied in `cs-tag`

Docs(Tag story): correct typo (#98)

Fixes typos in `cs-tag` story

Button a11y fixes (#97)

* Support enter keydown event
* Hide icons from screen readers in button stories

update modal and tests

update modal, test and stories

update tests

update styles

minor naming change

Renamed cs-drawer-width to simply width (#99)

Add "padding-line-between-statements" lint rule (#101)

Add Dropdown "quiet" variant (#96)

linting

update modal and stories
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ynotdraw ynotdraw ynotdraw approved these changes

@dylankcrwd dylankcrwd dylankcrwd approved these changes

@danwenzel danwenzel Awaiting requested review from danwenzel danwenzel is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@clintcs @ynotdraw @dylankcrwd