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

Add new Select component #828

Merged
merged 2 commits into from
Feb 8, 2023
Merged

Add new Select component #828

merged 2 commits into from
Feb 8, 2023

Conversation

lyzadanger
Copy link
Contributor

Depends on #827

This PR adds a basic Select component, building on InputRoot. Its purpose right now is to capture, standardize and simplify the design pattern used in the grading toolbar in LMS:

image

This is to support some UI improvements in that area.

The component is simple and young. You can see its documentation on this branch at http://localhost:4001/input-select

image

Comment on lines +16 to +18
// URI-encoded source of `CaretDownIcon`
// Currently, the color (stroke) is hard-coded
const arrowImage = `url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='16' height='16' viewBox='0 0 16 16' aria-hidden='true' focusable='false'%3E%3Cg fill-rule='evenodd'%3E%3Crect fill='none' stroke='none' x='0' y='0' width='16' height='16'%3E%3C/rect%3E%3Cpath fill='none' stroke='%239c9c9c' stroke-linecap='round' stroke-linejoin='round' stroke-width='2' d='M12 6l-4 4-4-4'%3E%3C/path%3E%3C/g%3E%3C/svg%3E")`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternately, we can extend the Tailwind theme in the package's preset:

theme: {
  extend: {
    backgroundImage: {
      'caret-down': 'url("/* ... *./")',
    }
  }
}

which could then be applied with the bg-caret-down utility class.

Putting the source of the image here in the component module means we could ostensibly set stroke color or other attribute values dynamically. The up side of putting it in the tailwind preset is that, ostensibly, a consumer could override it with something completely different.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there are pros and cons to both approaches, but since it's hard to see how the needs for this component will evolve, I would suggest keeping it like this and change it later if wee see the need.

Comment on lines +44 to +46
style={{
backgroundImage: arrowImage,
}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The LMS implementation of this <select> uses a separate Icon component/element and absolutely-positions it at the right edge of a wrapping container element.

The advantage of using a background image is that no wrapping container is needed. This allows <Select> to take advantage of InputGroup styling, e.g.

Comment on lines +16 to +18
// URI-encoded source of `CaretDownIcon`
// Currently, the color (stroke) is hard-coded
const arrowImage = `url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='16' height='16' viewBox='0 0 16 16' aria-hidden='true' focusable='false'%3E%3Cg fill-rule='evenodd'%3E%3Crect fill='none' stroke='none' x='0' y='0' width='16' height='16'%3E%3C/rect%3E%3Cpath fill='none' stroke='%239c9c9c' stroke-linecap='round' stroke-linejoin='round' stroke-width='2' d='M12 6l-4 4-4-4'%3E%3C/path%3E%3C/g%3E%3C/svg%3E")`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there are pros and cons to both approaches, but since it's hard to see how the needs for this component will evolve, I would suggest keeping it like this and change it later if wee see the need.

Base automatically changed from input-root to main February 8, 2023 18:08
@codecov
Copy link

codecov bot commented Feb 8, 2023

Codecov Report

Merging #828 (399a7fc) into main (9c1fbaa) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #828   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           57        58    +1     
  Lines          686       691    +5     
  Branches       241       242    +1     
=========================================
+ Hits           686       691    +5     
Impacted Files Coverage Δ
src/components/input/Select.tsx 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lyzadanger lyzadanger merged commit 861b63b into main Feb 8, 2023
@lyzadanger lyzadanger deleted the select-component branch February 8, 2023 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants