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

Feature/bulk download qr #1616

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

rockingrohit9639
Copy link
Contributor

Closes #1593

Copy link
Contributor

@DonKoko DonKoko left a comment

Choose a reason for hiding this comment

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

  • We have to do something about the performance of this. I did it on 100 assets and it took quite long. And knowing our users, I am pretty sure they will use it on like 1000 assets, all the time. I am not sure even where is the performance bottleneck so its good to figure it out.
  • When you click "Download" lets replace the content of the button with "Generating Zip file" + a spinner. This will make it more clear to people something is happening.
  • Can we look into adding some feedback in the modal, like a counter of how many codes are generated so far? We are doing this on the client so it should be quite straight forward.
  • For me the codes I downloaded are not styled. I tried both in Chrome and Firefox and got this:
    a2usebhh71nzchnahxpxw99zj

app/components/assets/bulk-download-qr-dialog.tsx Outdated Show resolved Hide resolved
app/components/assets/bulk-actions-dropdown.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@DonKoko DonKoko left a comment

Choose a reason for hiding this comment

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

  • Once the download finishes and the loading state is removed, you can still see the download button. This can be confusing for a lot of people. We should not show the download button when the download is finished

Performance

So i did a download of 390 codes and I notice some stuff I think we need to address. It took 1:30 minutes, which i guess is okey but I notice some other stuff that we should address. If you check the network tab you will see that for 390 codes, there were a lot of requests:
image
For example the font seems like its re-downloaded for each image. But then when I check the results, the CSS works now but the font doesn't:
Agilent 1260 Infinity II HPLC_i0zibgn68t
So we need to look into this. Maybe we have to load a web font or something, we have to check how to make this work. If for some reason we cant load our font or it keeps on adding so much overhead, maybe we can just use the device's native sans-serif font. Will still be better than the serif font it uses now.
I found this: tsayen/dom-to-image#90. Maybe its helpful. You can see here the creator of the font has it available on a cdn: https://rsms.me/inter/#usage

Copy link
Contributor

@DonKoko DonKoko left a comment

Choose a reason for hiding this comment

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

👌🏻
Deployed to staging for testing @carlosvirreira

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.

[Feature request]: Bulk QR Code Download from Asset Index
2 participants