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

Frontend Compatibility for Multiple Promoted Addon Groups #13343

Merged
merged 23 commits into from
Dec 10, 2024

Conversation

chrstinalin
Copy link
Contributor

@chrstinalin chrstinalin commented Dec 2, 2024

Closes mozilla/addons#15226

Makes the frontend compatible with receiving a list of promoted addons rather than just one, per mozilla/addons-server#22888.

Chooses to display only the most 'important' badge.

Still allows for just one promoted addon (i.e won't break anything while mozilla/addons-server#22888 is in progress)

Copy link

codecov bot commented Dec 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.28%. Comparing base (eb58fcd) to head (bb9b853).
Report is 61 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #13343   +/-   ##
=======================================
  Coverage   98.28%   98.28%           
=======================================
  Files         267      267           
  Lines       10620    10628    +8     
  Branches     3237     3241    +4     
=======================================
+ Hits        10438    10446    +8     
  Misses        169      169           
  Partials       13       13           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chrstinalin chrstinalin marked this pull request as ready for review December 3, 2024 17:15
@chrstinalin chrstinalin requested review from a team and KevinMind and removed request for a team December 3, 2024 17:18
KevinMind
KevinMind previously approved these changes Dec 5, 2024
Copy link
Contributor

@KevinMind KevinMind left a comment

Choose a reason for hiding this comment

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

I would approve this, with some nits, and verify with the addons-server PR after this is merged. wdyt?

src/amo/components/HeroRecommendation/index.js Outdated Show resolved Hide resolved
src/amo/utils/addons.js Outdated Show resolved Hide resolved
src/amo/utils/addons.js Outdated Show resolved Hide resolved
src/amo/components/AddonBadges/index.js Outdated Show resolved Hide resolved
@chrstinalin chrstinalin requested a review from KevinMind December 6, 2024 17:17
@willdurand willdurand self-requested a review December 9, 2024 14:35
@chrstinalin chrstinalin force-pushed the #14986-partner-addons-frontend branch from b99585a to f7c66f9 Compare December 9, 2024 14:53
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.

That's a good start, thanks. I provided some diffs to help you get to where I'd like us to go.

Additionally, I noticed a bunch of UI glitches:

  • For instance, how should we treat themes?
    Screenshot 2024-12-09 at 17 19 35

  • Badges in the search results should have some padding in between:
    Screenshot 2024-12-09 at 17 32 56

  • Multiple badges in the "more add-ons by this developer" card look a bit off:
    Screenshot 2024-12-09 at 17 35 19

  • same for "more themes by artist":
    Screenshot 2024-12-09 at 17 36 20

  • and in the user profile:
    Uploading Screenshot 2024-12-09 at 17.36.51.png…

  • and in the "other popular add-ons" card:
    Screenshot 2024-12-09 at 17 37 21

src/amo/components/HeroRecommendation/index.js Outdated Show resolved Hide resolved
src/amo/reducers/autocomplete.js Outdated Show resolved Hide resolved
tests/unit/amo/utils/test_addons.js Outdated Show resolved Hide resolved
tests/unit/amo/utils/test_addons.js Outdated Show resolved Hide resolved
@KevinMind KevinMind removed their request for review December 9, 2024 19:16
@KevinMind KevinMind requested review from KevinMind and willdurand and removed request for KevinMind December 9, 2024 19:16
@KevinMind
Copy link
Contributor

@willdurand idk what is happening but github refuses to let me remove myself, in the process of wrestling the UI I re-requested review from you (accidentally)

@KevinMind KevinMind self-requested a review December 9, 2024 19:18
@willdurand willdurand removed the request for review from KevinMind December 9, 2024 19:37
@chrstinalin chrstinalin dismissed willdurand’s stale review December 9, 2024 19:55

updated implementation to only show one badge in all scenarios

src/amo/reducers/autocomplete.js Outdated Show resolved Hide resolved
src/amo/utils/addons.js Outdated Show resolved Hide resolved
src/amo/utils/addons.js Outdated Show resolved Hide resolved
tests/unit/amo/pages/TestAddon.js Outdated Show resolved Hide resolved
tests/unit/amo/utils/test_addons.js Outdated Show resolved Hide resolved
tests/unit/amo/utils/test_addons.js Outdated Show resolved Hide resolved
src/amo/reducers/utils.js Show resolved Hide resolved
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.

Looking good, some more changes and we should be good

src/amo/utils/addons.js Outdated Show resolved Hide resolved
src/amo/utils/addons.js Outdated Show resolved Hide resolved
tests/unit/amo/reducers/test_utils.js Outdated Show resolved Hide resolved
tests/unit/amo/reducers/test_utils.js Show resolved Hide resolved
tests/unit/amo/reducers/test_utils.js Show resolved Hide resolved
tests/unit/amo/utils/test_addons.js Outdated Show resolved Hide resolved
tests/unit/amo/utils/test_addons.js Outdated Show resolved Hide resolved
@chrstinalin chrstinalin merged commit c55f660 into mozilla:master Dec 10, 2024
10 checks passed
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.

[Task]: Make addons-frontend Compatible with Multiple Promoted Addon Groups
3 participants