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

Privacy page simplification #14688

Merged
merged 16 commits into from
Jun 21, 2024

Conversation

wen-2018
Copy link
Collaborator

@wen-2018 wen-2018 commented Jun 14, 2024

One-line summary

  • I used an AI to write some of this code.

Significant changes and points to review

  • Simplify page content, remove unused assets and styles
  • update page content with new layout
  • update page styles
  • add new localization string
  • remove unused import and sub-nav
  • remove sub pages and their assets
  • remove obsolete ftl strings

Issue / Bugzilla link

#14248

Testing

http://localhost:8000/en-US/firefox/privacy/

@wen-2018 wen-2018 force-pushed the privacy-page-simplification branch from 8a5789c to dbd4a33 Compare June 14, 2024 19:15
@wen-2018 wen-2018 added the WIP 🚧 Pull request is still work in progress label Jun 17, 2024
@wen-2018 wen-2018 marked this pull request as ready for review June 17, 2024 18:50
@wen-2018 wen-2018 added Needs Review Awaiting code review Review: S Code review time: 30 mins to 1 hour P3 Third level priority - Nice to have and removed WIP 🚧 Pull request is still work in progress labels Jun 17, 2024
@stephaniehobson stephaniehobson self-assigned this Jun 17, 2024
Copy link
Contributor

@stephaniehobson stephaniehobson left a comment

Choose a reason for hiding this comment

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

Very through clean up. I love seeing all the code being removed 🧹

A couple quick changes and this'll be good to go.

bedrock/firefox/templates/firefox/privacy/index.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/privacy/index.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/privacy/index.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/privacy/index.html Outdated Show resolved Hide resolved
l10n/en/firefox/privacy-hub.ftl Outdated Show resolved Hide resolved
media/static-bundles.json Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/privacy/index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

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

There are also functional tests associated:

from pages.firefox.privacy.products import FirefoxPrivacyProductsPage
@pytest.mark.smoke
@pytest.mark.skip_if_firefox(reason="Download buttons are shown to non-Firefox browsers only")
@pytest.mark.nondestructive
def test_download_button_displayed(base_url, selenium):
page = FirefoxPrivacyProductsPage(selenium, base_url).open()

that ought to be removed with the page gone now.

Besides that only a couple of further cleanup ideas:

bedrock/firefox/redirects.py Show resolved Hide resolved
media/css/firefox/privacy/common.scss Outdated Show resolved Hide resolved
media/css/firefox/privacy/common.scss Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/privacy/index.html Outdated Show resolved Hide resolved
media/css/firefox/privacy/promise.scss Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/privacy/index.html Outdated Show resolved Hide resolved
l10n/en/firefox/privacy-hub.ftl Outdated Show resolved Hide resolved
@wen-2018 wen-2018 force-pushed the privacy-page-simplification branch from 5f71d26 to 172f9bc Compare June 19, 2024 18:02
Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.16%. Comparing base (7077dbc) to head (121485f).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14688      +/-   ##
==========================================
- Coverage   77.17%   77.16%   -0.01%     
==========================================
  Files         159      159              
  Lines        8245     8246       +1     
==========================================
  Hits         6363     6363              
- Misses       1882     1883       +1     

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

@stephaniehobson stephaniehobson merged commit 05b7857 into mozilla:main Jun 21, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review Awaiting code review P3 Third level priority - Nice to have Review: S Code review time: 30 mins to 1 hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants