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

Jump to bottom

Fixes #3900: Add file size info to addon's "More Info" section #8229

Merged
merged 2 commits into from Jul 2, 2019
Merged

Fixes #3900: Add file size info to addon's "More Info" section #8229

merged 2 commits into from Jul 2, 2019

Conversation

voronianski
Copy link
Contributor

@voronianski voronianski commented Jun 29, 2019 •
edited

Fixes #3900

This patch adds file size information to "More Info" section on addon's page.

Before:

Screenshot 2019-06-29 at 14 37 54

After:

Screenshot 2019-06-29 at 14 36 42

Approach and changes

  • updated AddonMoreInfo component with versionInfo prop
  • using getVersionInfo reducer for this purpose similarly to AddonVersionCard component which is already capable of displaying such data
  • added relevant test case into TestAddonMoreInfo to cover the change

@codecov-io
Copy link

codecov-io commented Jun 29, 2019 •
edited

Codecov Report

Merging #8229 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8229      +/-   ##
==========================================
+ Coverage   98.11%   98.11%   +<.01%     
==========================================
  Files         257      257              
  Lines        7328     7334       +6     
  Branches     1323     1325       +2     
==========================================
+ Hits         7190     7196       +6     
  Misses        124      124              
  Partials       14       14
Impacted Files Coverage Δ
src/amo/components/AddonMoreInfo/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75b1dee...09ce09f. Read the comment docs.

@voronianski voronianski changed the title Fixes #3000: Add file size to addon's "More Info" section Fixes #3900: Add file size to addon's "More Info" section Jun 29, 2019
@voronianski voronianski changed the title Fixes #3900: Add file size to addon's "More Info" section Fixes #3000: Add file size info to addon's "More Info" section Jun 29, 2019
@voronianski voronianski changed the title Fixes #3000: Add file size info to addon's "More Info" section Fixes #3900: Add file size info to addon's "More Info" section Jun 29, 2019
@willdurand willdurand self-assigned this Jul 1, 2019
willdurand
willdurand reviewed Jul 1, 2019
View reviewed changes
Copy link
Member

@willdurand willdurand left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

tests/unit/amo/components/TestAddonMoreInfo.js Outdated Show resolved Hide resolved
src/amo/components/AddonMoreInfo/index.js
translate(),
connect(mapStateToProps),
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the order because getVersionInfo reducer inside mapStateToProps needs i18n prop.

https://github.com/mozilla/addons-frontend/pull/8229/files#diff-4b306eb17d4142f5ba2588bb8ef72183R307

Copy link
Member

Choose a reason for hiding this comment

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

ah, I see. Thanks!

willdurand
willdurand approved these changes Jul 2, 2019
View reviewed changes
src/amo/components/AddonMoreInfo/index.js
translate(),
connect(mapStateToProps),
Copy link
Member

Choose a reason for hiding this comment

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

ah, I see. Thanks!

@willdurand willdurand merged commit ad2b84f into mozilla:master Jul 2, 2019
@willdurand
Copy link
Member

Thank you for your first contribution @voronianski 🎉 /cc @caitmuenster

@caitmuenster
Copy link

Woohoo! Thanks for the patch, @voronianski! ✨ Your contribution has been added to our recognition wiki and your mozillians profile has been vouched.

Welcome onboard! We look forward to seeing you around the project.

@voronianski
Copy link
Contributor Author

Woohoo! Thanks for the patch, @voronianski! ✨ Your contribution has been added to our recognition wiki and your mozillians profile has been vouched.

Welcome onboard! We look forward to seeing you around the project.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willdurand willdurand willdurand approved these changes

Assignees

@willdurand willdurand

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose the Addon size in the Add-on metadata
4 participants
@voronianski @codecov-io @willdurand @caitmuenster