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: Harmonize flagship upload view #3006

Merged
merged 2 commits into from
Oct 26, 2023

Conversation

acezard
Copy link
Contributor

@acezard acezard commented Oct 25, 2023

🐛 Bug Fixes

  • file icon
  • no back arrow
  • fix paddings
  • fix breadcrumb overlay

image

@bundlemon
Copy link

bundlemon bot commented Oct 25, 2023

BundleMon

Files updated (3)
Status Path Size Limits
drive/app/drive.(hash).js
274.32KB (+333B +0.12%) 280KB
drive/intents/drive.(hash).js
164.45KB (+25B +0.01%) 171KB
drive/public/drive.(hash).js
1.54MB (+20B 0%) 1.65MB
Unchanged files (16)
Status Path Size Limits
drive/vendors/drive.(hash).js
1.31MB 1.6MB
drive/public/pdf.worker.entry.(hash).worker.j
s
343.37KB 345KB
drive/services/dacc/drive.js
256.87KB 500KB
drive/services/qualificationMigration/drive.j
s
251.46KB 500KB
drive/public/cozy-client-js.js
158.97KB 160KB
drive/public/drive.(hash).min.css
92.4KB 100KB
drive/app-drive.(hash).min.css
55.52KB 56KB
drive/intents-drive.(hash).min.css
36.92KB 40KB
drive/onlyOffice/slide.pptx
24.82KB 25KB
drive/onlyOffice/text.docx
5.84KB 6KB
drive/onlyOffice/spreadsheet.xlsx
5.01KB 6KB
drive/manifest.webapp
1.88KB 2KB
drive/index.html
530B 1KB
drive/intents/index.html
515B 1KB
drive/img/app-icon.(hash).svg
419B 1KB
drive/manifest.json
184B 1KB

Total files change +378B +0.01%

Groups updated (3)
Status Path Size Limits
drive/app/**
274.32KB (+333B +0.12%) +10%
drive/intents/**
164.96KB (+25B +0.01%) +6%
drive/public/**
2.13MB (+20B 0%) +7%
Unchanged groups (5)
Status Path Size Limits
drive/screenshots/**
2.13MB +0.4%
drive/vendors/**
1.31MB +6%
drive/services/**
508.33KB +1%
drive/onlyOffice/**
35.68KB +0.4%
drive/img/**
419B +0.4%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@acezard acezard force-pushed the fix--Harmonize-flagship-upload-view-with-other-views branch 2 times, most recently from 730f732 to 53ad7f4 Compare October 25, 2023 14:22
- file icon
- no back arrow
- fix paddings
- fix breadcrumb overlay
@acezard acezard force-pushed the fix--Harmonize-flagship-upload-view-with-other-views branch from 53ad7f4 to 8fb8a54 Compare October 25, 2023 14:27
@acezard acezard requested a review from Ldoppea October 25, 2023 14:48
// The upload can only happen once as the webapp is closed before the upload process can start again
if (didFlow.current) return

const processed = (getProcessed(state) as () => unknown[]).length
Copy link
Member

Choose a reason for hiding this comment

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

This typing seems weird. Shouldn't this be getProcesses(state) as unknown[]? Otherwise you seems to call .length on a method pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's because I'm casting the actual getProcessed selector instead of the result, in any case it doesn't look great when using untyped code in strictly typed code

@@ -18,7 +18,8 @@ declare module 'cozy-ui/transpiled/react/providers/I18n' {

declare module 'cozy-ui/transpiled/react/deprecated/Alerter' {
const Alerter: {
error: (message: string) => void
error: (message: string, options?: Record<string, unknown>) => void
success: (message: string, options?: Record<string, unknown>) => void
Copy link
Member

Choose a reason for hiding this comment

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

Should we modify cozy-ui to expose those new definitions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes ideally, in this case I took the fastest way, I think it can be done after publication in test flight (for flagship too)

@@ -533,6 +533,7 @@
"header": "Uploading %{smart_count} photo to Cozy Drive |||| Uploading %{smart_count} photos to Cozy Drive",
"header_mobile": "Uploading %{done} of %{total}",
"header_done": "Uploaded %{done} out of %{total} successfully",
"success_flagship": "%{smart_count} file uploaded with success. |||| %{smart_count} files uploaded with success.",
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to add other languages translations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to do that in Transifex 👍

@acezard acezard merged commit 482affc into master Oct 26, 2023
3 checks passed
@acezard acezard deleted the fix--Harmonize-flagship-upload-view-with-other-views branch October 26, 2023 09:24
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.

2 participants