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

Upload: Improve asset cards and upload progress visual design #14849

Merged
merged 8 commits into from Nov 22, 2022

Conversation

gu-stav
Copy link
Contributor

@gu-stav gu-stav commented Nov 14, 2022

What does it do?

  • Fixes the badge alignment once @strapi/design-system@1.3.0 is released
Before After
Screenshot 2022-11-14 at 13 11 01 Screenshot 2022-11-14 at 13 12 49
  • Removes the border-radius bottom-left and bottom-right from UploadProgress
  • Updates UploadProgress colors for @strapi/design-system@1.3.0

Note
The attached PR updates the colors of the ProgressBar component, so that it will be visible again.

Before After
Screenshot 2022-11-14 at 13 09 14 Screenshot 2022-11-14 at 13 14 18

Why is it needed?

Stay up-to-date with the UI Kit.

Related issue(s)/PR(s)

@gu-stav gu-stav added source: core:upload Source is core/upload package pr: enhancement This PR adds or updates some part of the codebase or features labels Nov 14, 2022
@gu-stav
Copy link
Contributor Author

gu-stav commented Nov 14, 2022

@maevalienard The "cancel" button during upload doesn't have any hover/ focus effect: do you have an idea what we could use there?

@gu-stav gu-stav added the flag: don't merge This PR should not be merged at the moment label Nov 14, 2022
@codecov
Copy link

codecov bot commented Nov 14, 2022

Codecov Report

Base: 59.62% // Head: 59.58% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (2dbeca2) compared to base (21bdcdd).
Patch coverage: 80.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14849      +/-   ##
==========================================
- Coverage   59.62%   59.58%   -0.05%     
==========================================
  Files        1339     1339              
  Lines       32566    32608      +42     
  Branches     6207     6214       +7     
==========================================
+ Hits        19418    19428      +10     
- Misses      11289    11316      +27     
- Partials     1859     1864       +5     
Flag Coverage Δ
back 49.70% <ø> (?)
front 64.08% <80.00%> (-0.02%) ⬇️
unit ?
unit_back 49.70% <ø> (?)
unit_front 64.08% <80.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...min/src/components/AssetCard/UploadingAssetCard.js 35.29% <33.33%> (+1.96%) ⬆️
...ad/admin/src/components/AssetCard/AssetCardBase.js 100.00% <100.00%> (ø)
...pload/admin/src/components/UploadProgress/index.js 100.00% <100.00%> (ø)
packages/core/database/lib/utils/knex.js 62.50% <0.00%> (-17.50%) ⬇️
.../core/admin/admin/src/components/LeftMenu/index.js 26.47% <0.00%> (-3.04%) ⬇️
...ager/pages/ListSettingsView/components/Settings.js 73.52% <0.00%> (-1.48%) ⬇️
...core/strapi/lib/services/entity-validator/index.js 92.48% <0.00%> (-1.46%) ⬇️
packages/core/upload/server/services/metrics.js 62.26% <0.00%> (-0.70%) ⬇️
...e/database/lib/entity-manager/regular-relations.js 9.09% <0.00%> (-0.11%) ⬇️
...n/server/migrations/field/migrate-for-bookshelf.js 22.50% <0.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@ronronscelestes ronronscelestes left a comment

Choose a reason for hiding this comment

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

nice 💃
only comment, I feel like there is a misalignment between the cancel and the cross icon now 👀
image

@maevalienard
Copy link

The "cancel" button during upload doesn't have any hover/ focus effect: do you have an idea what we could use there?

Good catch. From what I saw, we could use neutral700 as hover color :)

@joshuaellis joshuaellis self-assigned this Nov 18, 2022
@joshuaellis
Copy link
Member

I've addressed the feedback from @maevalienard & addressed your comment @ronronscelestes – its now exactly as specified in the figma file 🙇🏼‍♀️

@joshuaellis joshuaellis removed the flag: don't merge This PR should not be merged at the moment label Nov 21, 2022
Copy link
Contributor

@ronronscelestes ronronscelestes left a comment

Choose a reason for hiding this comment

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

thanks for the fixes @joshuaellis 💃
one last UI tweak:

I think by doing that

<Card borderColor={error ? 'danger600' : undefined}>

in UploadingAssetCard we break the border color when no error

image

from my tests it should be fixed by doing:

<Card borderColor={error ? 'danger600' : 'neutral150'}>

Copy link
Contributor

@ronronscelestes ronronscelestes left a comment

Choose a reason for hiding this comment

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

looks goood 💅

@joshuaellis joshuaellis added this to the 4.5.2 milestone Nov 22, 2022
@alexandrebodin alexandrebodin removed this from the 4.5.2 milestone Nov 22, 2022
@joshuaellis joshuaellis added this to the 4.5.3 milestone Nov 22, 2022
@joshuaellis joshuaellis merged commit a73dc6f into main Nov 22, 2022
@joshuaellis joshuaellis deleted the fix/card-badge branch November 22, 2022 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: enhancement This PR adds or updates some part of the codebase or features source: core:upload Source is core/upload package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants