Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Dropdown): properly accept id on searchInput from Form #1938

Merged
merged 7 commits into from
Sep 19, 2019

Conversation

silviuaavram
Copy link
Collaborator

pass searchInput.id to Downshift's prop inputId.
fix the Form + Dropdown example to make the searcInput focusable on label click.

@silviuaavram silviuaavram added accessibility All the Accessibility tasks and bugs should be tagged with this. 🚀 ready for review labels Sep 16, 2019
@codecov
Copy link

codecov bot commented Sep 16, 2019

Codecov Report

Merging #1938 into master will decrease coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1938      +/-   ##
=========================================
- Coverage    70.4%   70.4%   -0.01%     
=========================================
  Files         892     892              
  Lines        7867    7870       +3     
  Branches     2295    2298       +3     
=========================================
+ Hits         5539    5541       +2     
- Misses       2315    2316       +1     
  Partials       13      13
Impacted Files Coverage Δ
...ackages/react/src/components/Dropdown/Dropdown.tsx 87.87% <66.66%> (-0.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47453b9...43fec52. Read the comment docs.

CHANGELOG.md Outdated
@@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Fix `CreateShorthandOptions` should be typed @lucivpav ([#1886](https://github.com/stardust-ui/react/pull/1886))
- Add `shadowLevel1Dark` in Teams themes @notandrew ([#1887](https://github.com/stardust-ui/react/pull/1887))
- When merging themes use deep merge for site and component variables @miroslavstastny ([#1907](https://github.com/stardust-ui/react/pull/1907))
- Fix `Dropdown` to properly accept `id` on `searchInpiut` @silviuavram ([#1938](https://github.com/stardust-ui/react/pull/1938))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Fix `Dropdown` to properly accept `id` on `searchInpiut` @silviuavram ([#1938](https://github.com/stardust-ui/react/pull/1938))
- Fix `Dropdown` to properly accept `id` on `searchInput` @silviuavram ([#1938](https://github.com/stardust-ui/react/pull/1938))

Typo

@@ -431,6 +431,7 @@ class Dropdown extends AutoControlledComponent<WithAsProp<DropdownProps>, Dropdo
onStateChange={this.handleStateChange}
labelId={this.props['aria-labelledby']}
environment={this.context.target.defaultView}
inputId={this.props.searchInput ? this.props.searchInput['id'] : undefined}
Copy link
Member

Choose a reason for hiding this comment

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

<Dropdown searchInput={render => render('boo')} />
<Dropdown searchInput='Oops' />

This thing is pretty fragile. Do we have other options?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you are right, I need to add another check. this will only work if searchInput is an object.

@vercel vercel bot temporarily deployed to staging September 17, 2019 13:23 Inactive
@silviuaavram silviuaavram merged commit 6d6c508 into master Sep 19, 2019
@silviuaavram silviuaavram deleted the docs/dropdown-form-label-click branch September 19, 2019 12:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility All the Accessibility tasks and bugs should be tagged with this. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants