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

Rebuild DropZone so it accesses ref.current instead of querySelector #8545

Merged
merged 2 commits into from
Mar 6, 2023

Conversation

alex-page
Copy link
Member

@alex-page alex-page commented Mar 3, 2023

WHY are these changes introduced?

This is needed when removing the useUniqId from v11.

WHAT is this pull request doing?

  • Replace addEventListener with useEventListener hook
  • Adjust types so that they work for all the different events
  • Removes the child class component and moves it's logic into the parent
  • Removes the open() getElement code and uses a ref for the input

How to 🎩

  • Thoroughly test the DropZone storybook examples in Firefox, Chrome and Safari
  • In particular make sure the Upload button works in all-components-dropzone--with-custom-file-dialog-trigger

🎩 checklist

@@ -35,6 +34,8 @@ import styles from './DropZone.scss';

export type DropZoneFileType = 'file' | 'image' | 'video';

type DropZoneEvent = Event | React.ChangeEvent<HTMLInputElement>;
Copy link
Member Author

Choose a reason for hiding this comment

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

This could be exported and used in utils/index.ts. For now it's duplicated 🤷

@alex-page alex-page changed the title Rebuild DropZone so it can useRef instead of getElement Rebuild DropZone so it accesses ref.current instead of querySelector Mar 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2023

size-limit report 📦

Path Size
polaris-react-cjs 220.91 KB (-0.05% 🔽)
polaris-react-esm 140.41 KB (-0.08% 🔽)
polaris-react-esnext 196.5 KB (-0.06% 🔽)
polaris-react-css 42.59 KB (0%)

Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

Looks good. 🎩 in all 3 browsers. I had noticed style discrepancies but realized they were unrelated.

@alex-page alex-page merged commit 7c174e4 into main Mar 6, 2023
@alex-page alex-page deleted the dropzone-useref branch March 6, 2023 22:32
Copy link
Member

@AndrewMusgrave AndrewMusgrave left a comment

Choose a reason for hiding this comment

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

Late to the party - nice work! 🙌

kyledurand pushed a commit that referenced this pull request Mar 7, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/[email protected]

### Minor Changes

- [#8546](#8546)
[`8872c0861`](8872c08)
Thanks [@MindRave](https://github.com/MindRave)! - Added "magic" color
to the Icon component's color prop type.


- [#8545](#8545)
[`7c174e47a`](7c174e4)
Thanks [@alex-page](https://github.com/alex-page)! - Updated DropZone
with a signifigant restructure to remove Class child component


- [#8525](#8525)
[`8a4de8168`](8a4de81)
Thanks [@rcaplanshopify](https://github.com/rcaplanshopify)! -
[IndexTable] Adds support for header config object alignment property
and treatment of right-aligned, sortable column headings


- [#8569](#8569)
[`646fba23f`](646fba2)
Thanks [@kyledurand](https://github.com/kyledurand)! - Allowed aria
attributes on Bleed, Inline, and Columns

### Patch Changes

- [#8581](#8581)
[`336d14545`](336d145)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed a bug where
bulk actions wouldn't render when only promoted actions exist

## @shopify/[email protected]



## [email protected]

### Patch Changes

- [#8570](#8570)
[`45ca38d41`](45ca38d)
Thanks [@laurkim](https://github.com/laurkim)! - Added `Legacy` status
to component lifecycle page and banner/badge to `LegacyStack` and
`LegacyCard`


- [#8573](#8573)
[`0389fd8b8`](0389fd8)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed links to
component lifecycle page in alpha component banners

- Updated dependencies
\[[`336d14545`](336d145),
[`8872c0861`](8872c08),
[`7c174e47a`](7c174e4),
[`8a4de8168`](8a4de81),
[`646fba23f`](646fba2)]:
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
alex-page added a commit that referenced this pull request Mar 8, 2023
### WHY are these changes introduced?

Fixes #8395 and is blocked by
#8545

### WHAT is this pull request doing?

Replaces `useUniqueId` with `useId`

---------

Co-authored-by: Aaron Casanova <[email protected]>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
…hopify#8545)

### WHY are these changes introduced?

This is needed when removing the `useUniqId` from v11.

### WHAT is this pull request doing?

- [x] Replace addEventListener with `useEventListener` hook
- [x] Adjust types so that they work for all the different events
- [x] Removes the child class component and moves it's logic into the
parent
- [x] Removes the `open()` `getElement` code and uses a ref for the
input

### How to 🎩

- [ ] Thoroughly test the DropZone storybook examples in Firefox, Chrome
and Safari
- [ ] In particular make sure the `Upload` button works in
`all-components-dropzone--with-custom-file-dialog-trigger`

### 🎩 checklist

- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Updated the component's `README.md` with documentation changes
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Aaron Casanova <[email protected]>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/[email protected]

### Minor Changes

- [Shopify#8546](Shopify#8546)
[`8872c0861`](Shopify@8872c08)
Thanks [@MindRave](https://github.com/MindRave)! - Added "magic" color
to the Icon component's color prop type.


- [Shopify#8545](Shopify#8545)
[`7c174e47a`](Shopify@7c174e4)
Thanks [@alex-page](https://github.com/alex-page)! - Updated DropZone
with a signifigant restructure to remove Class child component


- [Shopify#8525](Shopify#8525)
[`8a4de8168`](Shopify@8a4de81)
Thanks [@rcaplanshopify](https://github.com/rcaplanshopify)! -
[IndexTable] Adds support for header config object alignment property
and treatment of right-aligned, sortable column headings


- [Shopify#8569](Shopify#8569)
[`646fba23f`](Shopify@646fba2)
Thanks [@kyledurand](https://github.com/kyledurand)! - Allowed aria
attributes on Bleed, Inline, and Columns

### Patch Changes

- [Shopify#8581](Shopify#8581)
[`336d14545`](Shopify@336d145)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed a bug where
bulk actions wouldn't render when only promoted actions exist

## @shopify/[email protected]



## [email protected]

### Patch Changes

- [Shopify#8570](Shopify#8570)
[`45ca38d41`](Shopify@45ca38d)
Thanks [@laurkim](https://github.com/laurkim)! - Added `Legacy` status
to component lifecycle page and banner/badge to `LegacyStack` and
`LegacyCard`


- [Shopify#8573](Shopify#8573)
[`0389fd8b8`](Shopify@0389fd8)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed links to
component lifecycle page in alpha component banners

- Updated dependencies
\[[`336d14545`](Shopify@336d145),
[`8872c0861`](Shopify@8872c08),
[`7c174e47a`](Shopify@7c174e4),
[`8a4de8168`](Shopify@8a4de81),
[`646fba23f`](Shopify@646fba2)]:
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

4 participants