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

fix(blade): Trigger native select events in dropdown/ file upload / Date picker [FC-3151] #2408

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

tewarig
Copy link
Contributor

@tewarig tewarig commented Nov 6, 2024

Description

This pr add adds fixes -

  • Dropdown component does not fire change or input event when the value is changed or committed.
  • Date picker component does not fire change or input event when the value is changed or committed.
  • FileUpload component does not fire change or input event when the file is dropped.

for testing -
https://codesandbox.io/p/sandbox/razorpay-blade-basic-forked-ky8wfs?workspaceId=b3d29104-ffe4-442f-8d81-f2753e652d83

Changes

  • Adds a fireNativeEvent function
  • trigger fireNativeEvent function in DatePicker, useDropDown, fileUpload, BaseDropDownInputTrigger

Additional Information

Component Checklist

  • Update Component Status Page
  • Perform Manual Testing in Other Browsers
  • Add KitchenSink Story
  • Add Interaction Tests (if applicable)
  • Add changeset

Copy link

changeset-bot bot commented Nov 6, 2024

⚠️ No Changeset found

Latest commit: 82104ef

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Nov 6, 2024

✅ PR title follows Conventional Commits specification.

@tewarig tewarig changed the title Fix: trigger native events fix: trigger native events Nov 6, 2024
Copy link

codesandbox-ci bot commented Nov 6, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 82104ef:

Sandbox Source
razorpay/blade: basic Configuration

@rzpcibot
Copy link
Collaborator

rzpcibot commented Nov 6, 2024

Bundle Size Report

Updated Components
Status Component Base Size (kb) Current Size (kb) Diff
ActionList, ActionListItem, ActionListItemAvatar, ActionListItemBadge, ActionListItemBadgeGroup, ActionListItemIcon, ActionListItemText, ActionListSection 16.240 16.328 +0.088 KB
DatePicker 85.617 85.677 +0.060 KB
Dropdown, DropdownOverlay, DropdownButton, DropdownLink, DropdownFooter, DropdownHeader 28.214 28.284 +0.070 KB
FileUpload 17.317 17.397 +0.080 KB
Table, TableHeader, TableHeaderCell, TableHeaderRow, TableBody, TableCell, TableRow, TableFooter, TableFooterCell, TableFooterRow, TablePagination, TableToolbar, TableToolbarActions, TableEditableCell, TableEditableDropdownCell 65.921 66.030 +0.109 KB
SelectInput, AutoComplete 35.883 35.984 +0.101 KB
PhoneNumberInput 59.838 59.925 +0.087 KB
SearchInput 34.222 34.285 +0.063 KB

Generated by 🚫 dangerJS against 82104ef

@tewarig tewarig marked this pull request as ready for review November 7, 2024 20:22
@tewarig tewarig changed the title fix: trigger native events fix: Trigger native select events in dropdown/ file upload / Date picker Nov 8, 2024
@tewarig tewarig changed the title fix: Trigger native select events in dropdown/ file upload / Date picker fix: Trigger native select events in dropdown/ file upload / Date picker [FC-3151] Nov 8, 2024
@@ -158,6 +159,7 @@ const _FileUpload: React.ForwardRefRenderFunction<BladeElementRef, FileUploadPro
if (!hasValidationErrors) {
handleFilesChange(droppedFiles);
onDrop?.({ name, fileList: allFiles });
fireNativeEvent(inputRef, ['change', 'input']);
Copy link
Member

Choose a reason for hiding this comment

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

Check the normal DOM inputs if we actually fire both of these or not. I think they do but still confirm once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checked for <input type="file" /> they do fire both change and input .

@@ -176,6 +167,7 @@ const _BaseDropdownInputTrigger = (
defaultValue: props.defaultValue,
syncInputValueWithSelection: props.syncInputValueWithSelection,
isSelectInput: props.isSelectInput,
triggererRef,
Copy link
Member

Choose a reason for hiding this comment

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

@saurabhdaware can you check this once, not sure ye kaha se agaya

Copy link
Contributor Author

@tewarig tewarig Nov 12, 2024

Choose a reason for hiding this comment

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

i have added this in -
packages/blade/src/components/Input/DropdownInputTriggers/types.ts

Copy link
Member

@anuraghazra anuraghazra left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, Just add tests

@tewarig tewarig changed the title fix: Trigger native select events in dropdown/ file upload / Date picker [FC-3151] fix(blade): Trigger native select events in dropdown/ file upload / Date picker [FC-3151] Nov 11, 2024
@tewarig tewarig added the Waiting for Changes Waiting for changes from PR author label Nov 15, 2024
@anuraghazra
Copy link
Member

Can you check for the failing tests

@tewarig tewarig added changes done and removed Waiting for Changes Waiting for changes from PR author labels Nov 15, 2024
@tewarig
Copy link
Contributor Author

tewarig commented Nov 15, 2024

@anuraghazra changes done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants