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.

Sign up for GitHub

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

Jump to bottom

fix(input): update files property on input event when type="file" #11262

Merged
merged 3 commits into from
Jan 11, 2025
Merged

fix(input): update files property on input event when type="file" #11262

merged 3 commits into from
Jan 11, 2025

Conversation

benelan
Copy link
Member

@benelan benelan commented Jan 10, 2025

Related Issue: #9581

Summary

Ensure the files property is updated before calciteInputInput is emitted for type="file" inputs.

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jan 10, 2025
@benelan benelan added the skip visual snapshots Pull requests that do not need visual regression testing. label Jan 10, 2025
jcfranco
jcfranco approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Awesome! Would it be possible to add a test for this?

packages/calcite-components/src/components/input/input.tsx Show resolved Hide resolved
driskull
driskull approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

@benelan
Merge remote-tracking branch 'origin/dev' into benelan/9581-input-fil…
58ed5d9 
…e-event-timing

* origin/dev:
  chore: release next
  ci(renovate): group dep-bump PRs for @cspell packages (#11255)
  refactor(list): use debounce value from common debounce module instead of local one (#11259)
@benelan
chore: remove change handler
a53c948
@benelan
Copy link
Member Author

benelan commented Jan 11, 2025

Awesome! Would it be possible to add a test for this?

It wasn't possible with jest, but I did some research and vitest does have experimental support:
https://vitest.dev/guide/browser/interactivity-api.html#userevent-upload

We would need to add playwright or webdriverio to our vitest setup:
https://vitest.dev/guide/browser/#manual-installation

Would that play okay with Lumina @maxpatiiuk?

Maybe this warrants a separate issue @jcfranco?

@maxpatiiuk
Copy link
Member

maxpatiiuk commented Jan 11, 2025 •
edited
Loading

Would that play okay with Lumina @maxpatiiuk?

With Lumina yes, but:

I believe the documentation you are looking at is for vitest browser mode. Anything imported from @vitest/browser/ is for browser mode.

Migrating Calcite's E2E tests to vitest browser mode would be a DX improvement for sure, but it is much larger than the scope of this PR.

P.S: I couldn't see an issue for migration to vitest browser mode - should I create one?

@jcfranco
Copy link
Member

A new issue would be great, thanks! Just noting that transitioning to vitest browser mode will be put on hold until it is no longer experimental.

@benelan benelan merged commit eddf102 into dev Jan 11, 2025
10 checks passed
@benelan benelan deleted the benelan/9581-input-file-event-timing branch January 11, 2025 01:07
benelan added a commit that referenced this pull request Jan 11, 2025
@benelan
Merge remote-tracking branch 'origin/dev' into benelan/more-linting-i…
b3dd000 
…mprovements

* origin/dev:
  fix(input): update files property on input event when type="file" (#11262)
@maxpatiiuk maxpatiiuk mentioned this pull request Jan 12, 2025
Open
@maxpatiiuk
Copy link
Member

Opened an issue: #11268

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driskull driskull driskull approved these changes

@jcfranco jcfranco jcfranco approved these changes

@eriklharper eriklharper Awaiting requested review from eriklharper

@anveshmekala anveshmekala Awaiting requested review from anveshmekala

Assignees

@benelan benelan

Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. skip visual snapshots Pull requests that do not need visual regression testing.
Projects
None yet
Milestone
2025-01-30 - 3.0.0 Jan Breaking Chang...
Development

Successfully merging this pull request may close these issues.
4 participants
@benelan @maxpatiiuk @jcfranco @driskull