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

[select] fix: use renderTarget API to fix fill prop #5354

Merged
merged 10 commits into from
Jun 6, 2022

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Jun 3, 2022

Fixes #5353

Checklist

  • Includes tests - covered by existing test suite
  • Update documentation

Changes proposed in this pull request:

Select2, Suggest2, MultiSelect2:

  • Fix fill prop, which did not work and was a regression from the V1 components
  • This was done by migrating to the Popover2 renderTarget API, and has the added benefit of collapsing the DOM rendered by the components
    • ⚠️ This is a slightly breaking change, but we don't expect any real consumers using these component in production yet, so it's ok
  • Also improved the docs examples for these components so that they clearly demonstrate disabled, fill, matchTargetWidth props

Select2, MultiSelect2:

  • Fix matchTargetWidth styling so that the width of the popup menu is no longer constrained

MultiSelect2:

  • Add support for disabled?: boolean prop

Reviewers should focus on:

No regressions, passing test suite

Screenshot

2022-06-03 15 40 51

Screen Shot 2022-06-03 at 4 13 54 PM

@blueprint-bot
Copy link

revert margin change

Previews: documentation | landing | table | demo

@adidahiya adidahiya changed the title [select] fix(MultiSelect2): use renderTarget API to fix fill prop [select] fix: use renderTarget API to fix fill prop Jun 3, 2022
@blueprint-bot
Copy link

Fix typedef

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

fix test

Previews: documentation | landing | table | demo

@adidahiya
Copy link
Contributor Author

there's a bug with the min width of the popover when matchTargetWidth={true}:

image

@adidahiya
Copy link
Contributor Author

also disabled is not working for MultiSelect2:

image

@blueprint-bot
Copy link

Fix this.state.isOpen references

Previews: documentation | landing | table | demo

@adidahiya adidahiya merged commit 0c5823b into develop Jun 6, 2022
@adidahiya adidahiya deleted the ad/fix-multiselect2-fill branch June 6, 2022 22:07
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.

Fill prop doesn't work on MultiSelect 2 (when coming from MultiSelect)
2 participants