Skip to content

Generically type ButtonProps #290

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

Merged

Conversation

caryaharper
Copy link
Contributor

@caryaharper caryaharper commented Nov 16, 2021

This change requires that a type with the constraint of HTMLElement be passed to useDropdownMenu allowing any HTML element to be used as the “button”.

The purpose of this change is to allow the user to receive buttonProps tailored to their desired HTML element.

Some test were also updated to account for the new requirement of a type being passed to the hook.

Closes #69

@caryaharper caryaharper force-pushed the create-type-options-for-button-ref branch from 8602b96 to 7ba49c9 Compare November 16, 2021 23:12
@caryaharper caryaharper changed the title made hook use generic type to allow for any HTML element Generically type ButtonProps Nov 16, 2021
@codecov
Copy link

codecov bot commented Nov 16, 2021

Codecov Report

Merging #290 (08c6d54) into master (b086d98) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #290      +/-   ##
==========================================
+ Coverage   90.81%   90.90%   +0.09%     
==========================================
  Files           1        1              
  Lines          98       99       +1     
  Branches       32       32              
==========================================
+ Hits           89       90       +1     
  Misses          7        7              
  Partials        2        2              
Impacted Files Coverage Δ
src/use-dropdown-menu.ts 90.90% <100.00%> (+0.09%) ⬆️

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 b086d98...08c6d54. Read the comment docs.

@corymharper
Copy link
Contributor

This pull request should generate no changes to yarn.lock.

Copy link
Contributor

@corymharper corymharper left a comment

Choose a reason for hiding this comment

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

It looks like you need to add the generic type at website/src/pages/demo.tsx.

Copy link
Contributor

@corymharper corymharper left a comment

Choose a reason for hiding this comment

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

Can you introduce a generic type parameter default of HTMLButtonElement? Then a lot of the documentation won't have to change at all, along with some of the code, and we can add a page of documentation indicating how to use types other than HTMLButtonType for the menu button.

caryaharper and others added 3 commits January 17, 2022 16:29
Co-authored-by: Wes Cossick <WesCossick@users.noreply.github.com>
Co-authored-by: Wes Cossick <WesCossick@users.noreply.github.com>
…harper/react-accessible-dropdown-menu-hook into create-type-options-for-button-ref
@corymharper corymharper dismissed WesCossick’s stale review January 18, 2022 22:48

The requested changes have been made

Copy link
Member

@WesCossick WesCossick left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @caryaharper!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add support for new element types for menubutton element
3 participants