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

Dashboard: Add checks for logos dimensions #4313

Merged
merged 6 commits into from
Sep 4, 2020

Conversation

dmmulroy
Copy link
Contributor

@dmmulroy dmmulroy commented Sep 2, 2020

Summary

Adds error messages in the case that uploaded logos are smaller than 96x96.

User-facing changes

  • If you upload one image, and it is smaller than 96x96, the following error message will display: Sorry, this file's dimensions are too small. Make sure your logo is larger than 96x96.
  • If you upload two or more images, and one or more are smaller than 96x96, the following error message will display: Sorry, one or more of these files dimension's are too small. Make sure your logos are all larger than 96x96.

Testing Instructions

  • Upload a single image that is smaller than 96x96 and verify that you see the first error message above.
  • Upload two or more images, where at least one is smaller than 96x96, and verify that you see the second

#4147

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2020

Size Change: +254 B (0%)

Total Size: 1.24 MB

Filename Size Change
assets/js/stories-dashboard.js 583 kB +254 B (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 268 B 0 B
assets/css/stories-dashboard.css 305 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B
assets/js/chunk-web-stories-template-0-********************.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-1-********************.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-2-********************.js 10 kB 0 B
assets/js/chunk-web-stories-template-3-********************.js 10.5 kB 0 B
assets/js/chunk-web-stories-template-4-********************.js 10.6 kB 0 B
assets/js/chunk-web-stories-template-5-********************.js 6.86 kB 0 B
assets/js/chunk-web-stories-template-6-********************.js 9.91 kB 0 B
assets/js/chunk-web-stories-template-7-********************.js 9.78 kB 0 B
assets/js/edit-story.js 495 kB 0 B
assets/js/web-stories-activation-notice.js 63.6 kB 0 B
assets/js/web-stories-embed-block.js 16.9 kB 0 B

compressed-size-action

@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

Merging #4313 into main will increase coverage by 13.10%.
The diff coverage is 8.69%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #4313       +/-   ##
===========================================
+ Coverage   70.02%   83.12%   +13.10%     
===========================================
  Files         835      835               
  Lines       14728    14748       +20     
===========================================
+ Hits        10313    12260     +1947     
+ Misses       4415     2488     -1927     
Flag Coverage Δ
#karmatests 71.25% <8.69%> (+41.89%) ⬆️
#unittests 65.85% <8.69%> (-0.09%) ⬇️

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

Impacted Files Coverage Δ
...ts/src/dashboard/app/views/editorSettings/index.js 51.66% <0.00%> (-22.15%) ⬇️
assets/src/dashboard/constants/index.js 100.00% <100.00%> (ø)
...s/src/edit-story/components/panels/sizePosition.js 91.48% <0.00%> (+2.12%) ⬆️
assets/src/edit-story/elements/text/frame.js 97.50% <0.00%> (+2.50%) ⬆️
...edit-story/components/panels/panel/shared/title.js 81.08% <0.00%> (+2.70%) ⬆️
assets/src/edit-story/components/button/index.js 97.29% <0.00%> (+2.70%) ⬆️
...ponents/library/panes/media/media3p/media3pPane.js 93.75% <0.00%> (+3.12%) ⬆️
assets/src/edit-story/app/media/local/reducer.js 51.61% <0.00%> (+3.22%) ⬆️
...s/library/panes/media/media3p/media3pCategories.js 96.42% <0.00%> (+3.57%) ⬆️
assets/src/edit-story/app/media/local/actions.js 70.83% <0.00%> (+4.16%) ⬆️
... and 178 more

@carloskelly13
Copy link
Contributor

carloskelly13 commented Sep 2, 2020

Works well!

Screen Shot 2020-09-02 at 1 08 38 PM

Screen Shot 2020-09-02 at 1 08 54 PM

Copy link
Contributor

@carloskelly13 carloskelly13 left a comment

Choose a reason for hiding this comment

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

This is tricky to test but I think we can create a Mock Image object with a blob of random data and some metrics.

Copy link
Contributor

@mariano-formidable mariano-formidable left a comment

Choose a reason for hiding this comment

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

I agree with Pascal's error message changes, but functionally and code-ically it works! Nice job!!

@dmmulroy dmmulroy requested a review from swissspidy September 2, 2020 18:53
Copy link
Contributor

@BrittanyIRL BrittanyIRL left a comment

Choose a reason for hiding this comment

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

LGTM!

@dmmulroy dmmulroy merged commit a920cc1 into main Sep 4, 2020
@dmmulroy dmmulroy deleted the enhancement/4147-image-dims branch September 4, 2020 16:04
@swissspidy swissspidy added Group: Dashboard Pod: Pea (Dashboard & Templates) Type: Enhancement New feature or improvement of an existing feature labels Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Dashboard Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants