feat: add filled state to dropzone component #4617

rubencarvalho · 2024-07-23T09:42:59Z

Description

There is a new design for the dropzone component when it is in a "filled state". This PR adds this functionality to the component and respective documentation.

Related issue(s)

[Feat]: Dropzone filled+dragged state #3232

Motivation and context

Catching up the implementation with the most up to date designs.

How has this been tested?

Check the new filled state
1. Go to Storybook
2. Toggle the isFilled control.
Added integration test for the filled attribute

Screenshots

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

github-actions · 2024-07-23T09:45:07Z

Branch preview

Visual regression test results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

github-actions · 2024-07-23T09:48:38Z

Tachometer results

Chrome

dropzone permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	370 kB	14.40ms - 14.57ms	-	faster ✔ 1% - 3% 0.17ms - 0.40ms
branch	358 kB	14.69ms - 14.85ms	slower ❌ 1% - 3% 0.17ms - 0.40ms	-

Firefox

dropzone permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	370 kB	24.31ms - 27.33ms	-	unsure 🔍 -8% - +6% -2.03ms - +1.59ms
branch	358 kB	25.05ms - 27.03ms	unsure 🔍 -6% - +8% -1.59ms - +2.03ms	-

github-actions · 2024-07-23T12:16:35Z

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.99	0.99	0.95
Accessibility	1	1	1
Best Practices	1	1	1
SEO	1	0.92	0.92
PWA	1	1	1

What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category	Latest	Main	Branch
Total	222.803 kB	211.974 kB 🏆	212.271 kB
Scripts	54.878 kB	49.646 kB 🏆	49.874 kB
Stylesheet	34.67 kB	30.381 kB 🏆	30.432 kB
Document	6.076 kB	5.333 kB	5.333 kB
Font	127.179 kB	126.614 kB 🏆	126.632 kB

Request Count

Category	Latest	Main	Branch
Total	48	48	48
Scripts	40	40	40
Stylesheet	5	5	5
Document	1	1	1
Font	2	2	2

Rajdeepc · 2024-07-29T06:09:14Z

@rubencarvalho Can you update the hash and pull this up to main so that I can merge this for this release?

packages/dropzone/README.md

packages/dropzone/stories/dropzone.stories.ts

rubencarvalho · 2024-07-23T11:49:01Z

packages/dropzone/README.md

@@ -29,7 +29,7 @@ import { Dropzone } from '@spectrum-web-components/dropzone';
 ## Example

 ```html
-<sp-dropzone id="dropzone-1" style="width: 400px; height: 200px">
+<sp-dropzone id="dropzone-1" style="width: 400px;">


This fixed height was resulting in a broken design:

TarunAdobe · 2024-07-30T05:26:27Z

packages/dropzone/stories/dropzone.stories.ts

+export const Filled = (args: StoryArgs): TemplateResult => {
+    return html`
+        <sp-dropzone id="dropzone" ?filled=${args.isFilled}>
+            Filled dropzone
+        </sp-dropzone>
+    `;
+};
+Filled.args = {
+    isFilled: true,
+};
+


Probably nitpicking but can we make sure that all our stores look same? i.e., like the other stories of dropzone i'd like it to follow the same design with illustrated message and whatever else.
Also on the side since we try to (or atleast in theory we do) use the same template for all our stories to show different component states, it would be better if we can reuse the same template without rewriting the same code in stories. Not a blocker. Just wanted to get your opinion on making our codebase better.

That is a great point, and I think we should really consider a holistic approach to this (maybe outside the scope of this PR). But let me elaborate: since we use Storybook for both component demos and VRT, we need to balance showcasing component functionality with isolating each story's features to test specific functionalities. For example, if we change the sp-action-button or the sp-illustrated-message, it will cause sp-dropzone screenshots to fail. Is this acceptable? Maybe 🤷 However, ideally, we should have an additional layer of component demos that are independent of VRT so Storybook could focus solely on component testing in isolation.
I will follow-up on this.

This is a great discussion thread! We should definitely talk more on this. Currently the storybook config are tightly coupled to docs site demos and we are tightly coupled here! We are trying to create some obfuscated demos which will cater for Desktop and Mobile versions to be represented in docs site and are independent of storybook.
This looks like a good and informative way to represent component behaviours

TarunAdobe · 2024-07-30T05:36:56Z

Also @rubencarvalho you probably need to rebase this to main and update the golden image hash once again so that ci stops getting angry. Rest everything looks great. Thanks for working on it.

packages/action-menu/src/ActionMenu.ts

* feat: add filled state to dropzone component * chore: update golden images hash * chore: update dropzone controlled story to have reactivity * chore: update dropzone README to reflect interactivity * chore: update VRT hash * chore: update VRT hash

rubencarvalho force-pushed the feat-filled-dropzone branch from d5defec to 866a6a9 Compare July 23, 2024 09:45

rubencarvalho force-pushed the feat-filled-dropzone branch 2 times, most recently from 8af217f to 12626b7 Compare July 23, 2024 12:14

rubencarvalho force-pushed the feat-filled-dropzone branch from 5bec457 to af18c86 Compare July 25, 2024 12:30

rubencarvalho marked this pull request as ready for review July 25, 2024 12:31

rubencarvalho requested a review from a team July 26, 2024 08:19

Rajdeepc requested changes Jul 29, 2024

View reviewed changes

packages/dropzone/README.md Outdated Show resolved Hide resolved

packages/dropzone/stories/dropzone.stories.ts Outdated Show resolved Hide resolved

Rajdeepc added enhancement New feature or request Component: Dropzone labels Jul 29, 2024

rubencarvalho commented Jul 29, 2024

View reviewed changes

TarunAdobe requested a review from a team July 30, 2024 05:21

TarunAdobe requested changes Jul 30, 2024

View reviewed changes

blunteshwar reviewed Jul 30, 2024

View reviewed changes

packages/action-menu/src/ActionMenu.ts Outdated Show resolved Hide resolved

rubencarvalho force-pushed the feat-filled-dropzone branch 2 times, most recently from 6b2389a to a52f23a Compare July 30, 2024 08:57

rubencarvalho requested review from TarunAdobe, Rajdeepc and blunteshwar July 30, 2024 09:38

rubencarvalho added 6 commits July 30, 2024 12:17

feat: add filled state to dropzone component

3bbfa8d

chore: update golden images hash

7141ff8

chore: update dropzone controlled story to have reactivity

49d9ac8

chore: update dropzone README to reflect interactivity

1cd57fb

chore: update VRT hash

39c8f38

chore: update VRT hash

65985ef

rubencarvalho force-pushed the feat-filled-dropzone branch from 6e46953 to 65985ef Compare July 30, 2024 10:17

Rajdeepc approved these changes Jul 30, 2024

View reviewed changes

Rajdeepc merged commit f6b7144 into main Jul 30, 2024
58 checks passed

Rajdeepc deleted the feat-filled-dropzone branch July 30, 2024 12:09

najikahalsema mentioned this pull request Aug 7, 2024

[Feat]: Dropzone filled+dragged state #3232

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add filled state to dropzone component #4617

feat: add filled state to dropzone component #4617

rubencarvalho commented Jul 23, 2024 •

edited by Rajdeepc

Loading

github-actions bot commented Jul 23, 2024

github-actions bot commented Jul 23, 2024 •

edited

Loading

dropzone permalink

test-basic

dropzone permalink

test-basic

github-actions bot commented Jul 23, 2024 •

edited

Loading

Rajdeepc commented Jul 29, 2024

rubencarvalho Jul 23, 2024

TarunAdobe Jul 30, 2024

rubencarvalho Jul 30, 2024

Rajdeepc Jul 30, 2024 •

edited

Loading

TarunAdobe commented Jul 30, 2024

feat: add filled state to dropzone component #4617

feat: add filled state to dropzone component #4617

Conversation

rubencarvalho commented Jul 23, 2024 • edited by Rajdeepc Loading

Description

Related issue(s)

Motivation and context

How has this been tested?

Screenshots

Types of changes

Checklist

Best practices

github-actions bot commented Jul 23, 2024

Branch preview

github-actions bot commented Jul 23, 2024 • edited Loading

Tachometer results

dropzone permalink

test-basic

dropzone permalink

test-basic

github-actions bot commented Jul 23, 2024 • edited Loading

Lighthouse scores

Transfer Size

Request Count

Rajdeepc commented Jul 29, 2024

rubencarvalho Jul 23, 2024

Choose a reason for hiding this comment

TarunAdobe Jul 30, 2024

Choose a reason for hiding this comment

rubencarvalho Jul 30, 2024

Choose a reason for hiding this comment

Rajdeepc Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

TarunAdobe commented Jul 30, 2024

rubencarvalho commented Jul 23, 2024 •

edited by Rajdeepc

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

Rajdeepc Jul 30, 2024 •

edited

Loading