Skip to content

Conversation

oksanashopify
Copy link
Contributor

@oksanashopify oksanashopify commented Feb 13, 2024

While working on new Variants card on PDP, I've noticed that Drop zone automatically calculates the size of the node it’s being applied and defines wether it should be Large/Medium/Small sized drop zone. The problem I faced - it sets $dropzone-min-height-small: 50px; which doesn’t work for thumbnails of size "small", as they have a height of 40px (--p-height-1000).

I considered several possible solutions, suggested in thread here:

  1. Ship changes to Polaris DropZone and prevent automatically calculates the size of area so it can be regulated on consumer level by setting height and width of parent block. But since Polaris drop zone is being used across 37 files within the Admin and it's size not always is regulated by parent block, like for example (UploadApplePayCertificateModals, ImagePicker, etc), also even if there is already fixed height for parent block, places like this should be revised across the Admin too, because as soon as we remove min-height - some of the DropZones could be broken until we update web code. Example in sandbox: https://screenshot.click/13-45-xs5hg-mkfsk.mp4

  2. Second possible solutions I can see here - adjust dropzone's small size to be the same as thumbnail small size, by setting it's min-height to 40px. It seems to be a change for 10 px, but now it's going to fit small sized thumbnails - Proposal Implemented in this PR

Resolves issue: https://github.com/Shopify/temp-project-mover-syfiawoo-20250331132715/issues/649

WHY are these changes introduced?

The dropzone's small size has been adjusted to match the small size of the thumbnail.

WHAT is this pull request doing?

How to 🎩

How it was before:
image

How it Looks now:
image

Updated Polaris website as well:
image

My Spin with applied snapshot: https://admin.web.oa-variant-image-dropzone-2.oksana-azarova.us.spin.dev/store/shop1/products/1

image

You can also test it on your own spin, just make sure admin_pdp_variants_250_ux beta is On and you created your branch from oa-variant-image-dropzone branch

Steps:

  1. Created product with several options, that will result on grouped variants view
  2. Expand one of the grouped rows and make sure drop zone does not expands table row and doesn't have height bigger that a small thumbnail.

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@oksanashopify oksanashopify marked this pull request as draft February 13, 2024 16:21
@oksanashopify oksanashopify self-assigned this Feb 13, 2024
@oksanashopify
Copy link
Contributor Author

/snapit

@oksanashopify oksanashopify marked this pull request as ready for review February 13, 2024 17:18
Copy link
Contributor

🫰✨ Thanks @oksanashopify! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

yarn add @shopify/polaris@0.0.0-snapshot-release-20240213171838

@oksanashopify oksanashopify marked this pull request as draft February 13, 2024 17:28
@oksanashopify oksanashopify marked this pull request as ready for review February 13, 2024 17:46
Copy link
Contributor

@tatianau tatianau left a comment

Choose a reason for hiding this comment

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

Found an issue with content jump

$dropzone-min-height-large: 120px;
$dropzone-min-height-medium: 100px;
$dropzone-min-height-small: 50px;
$dropzone-min-height-small: 40px;
Copy link
Contributor

Choose a reason for hiding this comment

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

When testing admin spin, I noticed there is a content jump which is an issue. Is that something that we have to control at the consumer level or polaris dropzone? Can this be fixed at consumer or polaris for tophatting before we merge this to ensure we fix it at the source of the problem?

My.Movie.mp4

Copy link
Contributor Author

@oksanashopify oksanashopify Feb 13, 2024

Choose a reason for hiding this comment

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

Thank you @tatianau for catching this! I fixed it on consumer side by adding width and height to parent block.
How it works now:

Screen.Recording.2024-02-13.at.1.00.06.PM.mov

I still can see that images/thumbnails kinda take some time to be rendered, looking further if it's something with my implementation.

Copy link
Contributor Author

@oksanashopify oksanashopify Feb 13, 2024

Choose a reason for hiding this comment

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

@tatianau you were right, that delay with image rendering is caused by drop zone, here is sandbox link
Video:

Screen.Recording.2024-02-13.at.2.56.50.PM.mov

I'l surface it to Veronica and Polaris team

@oksanashopify oksanashopify requested review from a team and tatianau February 13, 2024 20:50
@sam-b-rose
Copy link
Member

/snapit

Copy link
Contributor

🫰✨ Thanks @sam-b-rose! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

yarn add @shopify/polaris@0.0.0-snapshot-release-20240213211805

return (
<div style={{width: 50, height: 50}}>
<div style={{width: 40, height: 40}}>
<DropZone>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated example on Polaris website as well.
One thing I'm not sure about - locally it shows old styles. (screenshot attached). The change you, @sam-b-rose were asking bout
seems has been not applied.
How do we update Polaris website with new version of Polaris? Is there something I should do to update it locally?
Screenshot 2024-02-13 at 3 44 46 PM

Copy link
Member

Choose a reason for hiding this comment

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

You may need to run yarn build --filter="@shopify/polaris" before running the site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! That helped!

@oksanashopify
Copy link
Contributor Author

/snapit

Copy link
Contributor

🫰✨ Thanks @oksanashopify! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

yarn add @shopify/polaris@0.0.0-snapshot-release-20240214151548

@oksanashopify oksanashopify merged commit f829ed4 into main Feb 14, 2024
@oksanashopify oksanashopify deleted the oa-change-dropzone-min-height branch February 14, 2024 15:32
sam-b-rose pushed a commit that referenced this pull request Feb 15, 2024
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/polaris-icons@8.3.0

### Minor Changes

- [#11526](#11526)
[`b65f1e679`](b65f1e6)
Thanks [@j-wanita](https://github.com/j-wanita)! - Add list icons for
product, collection, metaobject and text


- [#11531](#11531)
[`78ed5fe0d`](78ed5fe)
Thanks [@j-wanita](https://github.com/j-wanita)! - Updated metaobject,
metaobject reference, and metaobject filled icons

## @shopify/polaris-migrator@0.28.0

### Minor Changes

- [#11596](#11596)
[`c8fabc011`](c8fabc0)
Thanks [@lgriffee](https://github.com/lgriffee)! - Created migration to
replace deprecated `font` custom properties in polaris-react v13.0.0

### Patch Changes

- [#11603](#11603)
[`2c53d6476`](2c53d64)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated migration to
replace deprecated `font` custom properties in polaris-react v13.0.0

## @shopify/polaris@12.16.0

### Minor Changes

- [#11585](#11585)
[`1ba3181b6`](1ba3181)
Thanks [@tauthomas01](https://github.com/tauthomas01)! - Added a
`disabled` prop to `ResourceItem`


- [#11568](#11568)
[`525194767`](5251947)
Thanks [@mrcthms](https://github.com/mrcthms)! - Updated the stacking
logic of multiple Toasts to take up less screen real estate


- [#11587](#11587)
[`5ab254b3b`](5ab254b)
Thanks [@sainihas](https://github.com/sainihas)! - Update dropzone
container background color when no Outline

### Patch Changes

- [#11581](#11581)
[`47dac1b2e`](47dac1b)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed an issue
where scrollbars weren't showing up in IndexTable on mac os when show
when scrolling preference is selected


- [#11560](#11560)
[`0b38b6115`](0b38b61)
Thanks [@apliano](https://github.com/apliano)! - Fixed `Combobox` not
rendering `Popover` until the second firing of the `onChange` event


- [#11584](#11584)
[`23d8297ff`](23d8297)
Thanks [@mrcthms](https://github.com/mrcthms)! - Updated
`useIsSelectAllActionsSticky` logic to not set any sticky behaviour if
we do not have access to the root element


- [#11543](#11543)
[`165bc6eae`](165bc6e)
Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed `IndexFilters`
height changing when toggling between default and filtering modes


- [#11563](#11563)
[`3937739d2`](3937739)
Thanks [@chloerice](https://github.com/chloerice)! - Fixed
`FormLayout.Item` overflowing viewport at xs breakpoint when user
settings enlarge text size


- [#11595](#11595)
[`f829ed487`](f829ed4)
Thanks [@oksanashopify](https://github.com/oksanashopify)! - Updated
DropZone minimum size from 50px to 40px to fit within a small Thumbnail

- Updated dependencies
\[[`b65f1e679`](b65f1e6),
[`78ed5fe0d`](78ed5fe)]:
    -   @shopify/polaris-icons@8.3.0

## polaris.shopify.com@0.63.0

### Minor Changes

- [#11596](#11596)
[`c8fabc011`](c8fabc0)
Thanks [@lgriffee](https://github.com/lgriffee)! - Created migration to
replace deprecated `font` custom properties in polaris-react v13.0.0


- [#11568](#11568)
[`525194767`](5251947)
Thanks [@mrcthms](https://github.com/mrcthms)! - Updated the stacking
logic of multiple Toasts to take up less screen real estate

### Patch Changes

- Updated dependencies
\[[`b65f1e679`](b65f1e6),
[`47dac1b2e`](47dac1b),
[`0b38b6115`](0b38b61),
[`23d8297ff`](23d8297),
[`1ba3181b6`](1ba3181),
[`165bc6eae`](165bc6e),
[`78ed5fe0d`](78ed5fe),
[`3937739d2`](3937739),
[`f829ed487`](f829ed4),
[`525194767`](5251947),
[`5ab254b3b`](5ab254b)]:
    -   @shopify/polaris-icons@8.3.0
    -   @shopify/polaris@12.16.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
…f the thumbnail. (Shopify#11595)

While working on new Variants card on PDP, I've noticed that [Drop
zone](https://polaris.shopify.com/components/selection-and-input/drop-zone?example=drop-zone-small-sized)
automatically calculates the size of the node it’s being applied and
defines wether it should be Large/Medium/Small sized drop zone. The
problem I faced - it sets $dropzone-min-height-small: 50px; which
doesn’t work for thumbnails of size "small", as they have a height of
40px (--p-height-1000).

I considered several possible solutions, suggested in thread
[here](https://shopify.slack.com/archives/C4Y8N30KD/p1707762462537299):
1. Ship changes to Polaris DropZone and prevent automatically calculates
the size of area so it can be regulated on consumer level by setting
height and width of parent block. But since Polaris drop zone is being
used across 37 files within the Admin and it's size not always is
regulated by parent block, like for example
([UploadApplePayCertificateModals](https://github.com/Shopify/web/blob/4a0de2c34d7cb55ad499e8e2e60d5e463bb5c61d/app/sections/Apps/AppManage/components/AppCapabilities/components/StorefrontApiIntegration/components/IOSBuySDKCard/ApplePay/components/UploadApplePayCertificateModals/UploadApplePayCertificateModals.tsx#L164),
[ImagePicker](https://github.com/Shopify/web/blob/4a0de2c34d7cb55ad499e8e2e60d5e463bb5c61d/app/sections/Checkout/CheckoutEditor/components/Workspace/components/Branding/components/BrandingSettingRenderer/components/BrandingImageSetting/components/ImagePicker/ImagePicker.tsx#L160),
etc), also even if there is already fixed height for parent block,
places like this should be revised across the Admin too, because as soon
as we remove `min-height `- some of the DropZones could be broken until
we update `web` code. Example in sandbox:
https://screenshot.click/13-45-xs5hg-mkfsk.mp4

2. Second possible solutions I can see here - adjust dropzone's small
size to be the same as
[thumbnail](https://polaris.shopify.com/components/images-and-icons/thumbnail)
small size, by setting it's min-height to 40px. It seems to be a change
for 10 px, but now it's going to fit small sized thumbnails - **Proposal
Implemented in this PR**

Resolves issue: https://github.com/Shopify/polaris/issues/11600

### WHY are these changes introduced?

The dropzone's small size has been adjusted to match the small size of
the thumbnail.


<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

  Include a video if your changes include interactive content.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

  <details>
    <summary>Summary of your gif(s)</summary>
    <img src="..." alt="Description of what the gif shows">
  </details>
-->

### How to 🎩
How it was before:

![image](https://github.com/Shopify/polaris/assets/104942025/04f9d9f7-5a23-4297-b92b-4469eb6bd969)


How it Looks now:
<img width="785" alt="image"
src="https://github.com/Shopify/polaris/assets/104942025/9257d933-8eaf-4584-8b40-77e9a38bf2b8">

Updated Polaris website as well: 
<img width="959" alt="image"
src="https://github.com/Shopify/polaris/assets/104942025/d752e831-c713-4367-ab37-022367e717c5">


My Spin with applied snapshot:
https://admin.web.oa-variant-image-dropzone-2.oksana-azarova.us.spin.dev/store/shop1/products/1

<img width="1806" alt="image"
src="https://github.com/Shopify/polaris/assets/104942025/b08c074d-22fd-44da-8e0c-36e2eac1494a">

You can also test it on your own spin, just make sure
`admin_pdp_variants_250_ux` beta is On and you created your branch from
`oa-variant-image-dropzone` branch

Steps:

1. Created product with several options, that will result on grouped
variants view
2. Expand one of the grouped rows and make sure drop zone does not
expands table row and doesn't have height bigger that a small thumbnail.



🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [x] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [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: Sam Rose <11774595+sam-b-rose@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.

3 participants