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

feat: add FileUpload component #2016

Merged
merged 31 commits into from
Feb 28, 2024
Merged

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Feb 14, 2024

Copy link

changeset-bot bot commented Feb 14, 2024

🦋 Changeset detected

Latest commit: e126797

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@razorpay/blade Minor

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Contributor

github-actions bot commented Feb 18, 2024

✅ PR title follows Conventional Commits specification.

Copy link

codesandbox-ci bot commented Feb 19, 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 e126797:

Sandbox Source
razorpay/blade: basic Configuration

@snitin315 snitin315 force-pushed the feat/add-file-upload branch from 9068984 to 1bca91e Compare February 20, 2024 05:10
@snitin315 snitin315 marked this pull request as ready for review February 23, 2024 05:32
@snitin315 snitin315 added the Review - L1 First level of review label Feb 23, 2024
@anuraghazra
Copy link
Member

There's a quite a lot of layout shift which is happening after selecting a file, this is happening because after the "Upload GST" label the margin is increasing, ideally margin should stay same.

Screen.Recording.2024-02-23.at.2.56.42.PM.mov

On design the margin stays consistent:

image

@anuraghazra
Copy link
Member

File name and the Tick icon doesn't look aligned vertically.

image

On design:

image

@anuraghazra
Copy link
Member

anuraghazra commented Feb 23, 2024

There's a gap in the bottom after the progressbar:

image
image

Gap not present in design:

image

@anuraghazra
Copy link
Member

In the first storybook example, can you wire up the controls? I cannot play around with different variants like isDisabled.

@snitin315
Copy link
Member Author

File name and the Tick icon doesn't look aligned vertically.

Fixed

In the first storybook example, can you wire up the controls? I cannot play around with different variants like isDisabled.

Done

Check once with RK on how it looks in mobile:

Updated

Tried on my android, truncation fails and text overflows for some reason:

Can you try again? I think this is the same as #2016 (comment)

There's a gap in the bottom after the progressbar:

Fixed.

@anuraghazra

@anuraghazra
Copy link
Member

File name and the Tick icon doesn't look aligned vertically.

Weird, cannot find any stories which shows the tick mark anymore, Unable to confirm this.

There's a gap in the bottom after the progressbar:

The file upload progress story doesn't show the progressbar anymore upon selecting the file, Unable to confirm this.

@anuraghazra
Copy link
Member

this one still exists.

@anuraghazra
Copy link
Member

labelPosition: left looks broken:

image

useMemo(() => {
for (const file of selectedFiles) {
if (!file.id) {
file.id = `${new Date().getTime().toString()}${Math.floor(Math.random() * 1000000)}`;
Copy link
Member

Choose a reason for hiding this comment

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

can we not use our useId('file-upload-id') hook?

@snitin315
Copy link
Member Author

@anuraghazra Fixed the stories, you can check again.

this #2016 (comment) still exists.

Fixed

labelPosition: left looks broken:

It's the current behavior of FormLabel for all our inputs. The * goes into the next line.


Screenshot 2024-02-28 at 8 26 07 AM

@saurabhdaware
Copy link
Member

The examples in stories and examples on stackblitz seem like same examples. Maybe you can keep basic examples in stories and move complex examples in stackblitz because they'll be able to see full code on stackblitz

@snitin315
Copy link
Member Author

The examples in stories and examples on stackblitz seem like same examples. Maybe you can keep basic examples in stories and move complex examples in stackblitz because they'll be able to see full code on stackblitz

Stackblitz takes significant boot-up time hence I've kept both. Users can play around with stories and look at examples if they want to see the implementation.

@snitin315 snitin315 added Review - L2 Second level of review and removed Review - L1 First level of review labels Feb 28, 2024
@snitin315 snitin315 changed the base branch from master to file-upload-integration February 28, 2024 06:02
@snitin315 snitin315 merged commit 4fdf562 into file-upload-integration Feb 28, 2024
14 checks passed
@snitin315 snitin315 deleted the feat/add-file-upload branch February 28, 2024 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review - L2 Second level of review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants