-
Notifications
You must be signed in to change notification settings - Fork 549
Add Policies FAQ to links and restructure links to make them more obvious #23689
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
base: master
Are you sure you want to change the base?
Conversation
8934c3b
to
4a67ec2
Compare
Since this is an important page for us, would you mind adding a screenshot of what it looks like with your patch applied? |
It looks like we did that, but only kind of. We updated the base domain, but the paths are also different. For instance, https://extensionworkshop.com/AMO/Policy/FAQ/ does not exist. |
4a67ec2
to
f71d8ec
Compare
@wagnerand added a video demonstrating the changes. LMK what you think. I know it is not precisely the ask from the ticket, but I believe it is better.. HTML check boxes are supposed to be clickable via their label. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR restructures the developer agreement form by moving documentation links into explanatory text and improving the form's usability. The changes address the issue of confusing HTML structure when adding new policy links.
- Restructured documentation URL mapping to focus on policy-related pages and updated redirect targets
- Modified the agreement template to include policy links in explanatory text rather than form labels
- Enhanced form accessibility by converting checkbox labels to proper HTML label elements
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/olympia/devhub/views.py | Updated docs function to use developer_docs mapping with Extension Workshop URLs and added context variables for policy links |
src/olympia/devhub/tests/test_views.py | Updated test cases to reflect the reduced set of documentation URLs |
src/olympia/devhub/templates/devhub/includes/agreement.html | Restructured agreement form with policy links in explanatory text and proper HTML labels for checkboxes |
This comment was marked as resolved.
This comment was marked as resolved.
@fjosephmoz can you take a look at the video above and let us know if you are ok with the design and copy? |
4013fb2
to
c0b237e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code review r+wc
- I'll check it out for a test once Frantz has okayed the copy, etc.
src/olympia/devhub/views.py
Outdated
if doc_name in mdn_docs: | ||
return redirect(MDN_BASE + mdn_docs[doc_name], permanent=True) | ||
if doc_name in developer_docs: | ||
return redirect( | ||
settings.EXTENSION_WORKSHOP_URL + developer_docs[doc_name], | ||
permanent=True, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not against dropping these ancient MDN redirects per-sea ... but this are hidden changes not even mentioned in the description, let alone discussed and agreed to in the issue.
My preference would be dropping this and dealing with it in a separate issue (i.e. keep the MDN redirects for everything other than the 3 new ones we're redirecting to EW), but at the very least it should be documented in this PR and have Andreas or Alan confirm they're okay with using dropping these old redirects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would lean towards dropping them and seeing if we get any 404s but I checked which of them are still working
requesting https://developer.mozilla.org/en-US/Add-ons
requesting https://developer.mozilla.org/en-US/Add-ons
requesting https://developer.mozilla.org/en-US/Add-ons
requesting https://developer.mozilla.org/en-US/Add-ons
requesting https://developer.mozilla.org/en-US/Add-ons
requesting https://developer.mozilla.org/en-US/Add-ons#Extensions
requesting https://developer.mozilla.org/en-US/Add-ons#Other_types_of_add-ons
requesting https://developer.mozilla.org/en-US/Add-ons#Application-specific
requesting https://developer.mozilla.org/en-US/Add-ons#Themes
requesting https://developer.mozilla.org/en-US/Add-ons/Themes/Background
/Themes/Background failed
requesting https://developer.mozilla.org/en-US/Add-ons/Themes/Background/FAQ
/Themes/Background/FAQ failed
requesting https://developer.mozilla.org/en-US/Add-ons/AMO/Policy
requesting https://developer.mozilla.org/en-US/Add-ons/AMO/Policy/Reviews
requesting https://developer.mozilla.org/en-US/Add-ons/AMO/Policy/Contact
/AMO/Policy/Contact failed
requesting https://developer.mozilla.org/en-US/Add-ons/AMO/Policy/Agreement
Results are:
- 3 are already 404ing on MDN
- 3 are redirecting to extensionworkshop
- 1 links to an actual subsection of the addons page on MDN
- all the rest (8) just show the same page on MDN.
I'll revert the change but we are keeping stuff around for no reason and we shouldn't do that.
Just looked at the video and I'd like to go back to the original layout.
Just a note - I want us to incentivize people to click on and read those links through design. this treatment seems like it would be less likely for a new developer to read the policies or the distro agreement from the UX perspective. Since the link to the info and the action to click are separated and not gated. (i.e., I don't have to click the links in the paragraph to check the boxes). |
c0b237e
to
2a2fc4d
Compare
- Replaced hardcoded URLs in the `docs` view with helper functions for better maintainability. - Updated the agreement form in the template to use labels for inputs, improving accessibility. - Adjusted the test for agreement form errors to match the new HTML structure. - Added a redirect test for the policies documentation page.
2a2fc4d
to
0fac43b
Compare
@fjosephmoz That's a good point. making the link integrated to the checkbox effectively requires the developer to open the link. One of my concerns with that approach was the fact we will (in the new version) now have 2 links on the same line. In the ticket we have the red text but in production we would have the same blue. There will be no way for a developer to understand that it is 2 links. It breaks the assumption that we "guarantee" they've opened the links and is also super confusing. Could a hybrid approach be to hoist the FAQ link to the description paragraph (with full text like you suggested) and then we can keep the original links as they are one per line? Wdyt? |
- Modified the agreement form template to use anchor tags for documentation links, enhancing user experience and accessibility. - Removed outdated test cases related to non-existent documentation pages to streamline the test suite.
Fixes: mozilla/addons#15681
Description
Refactor documentation links and update agreement form layout
docs
view with helper functions for better maintainability.Context
We need to add another link to an already confusing html structure and we mention the "exact" same words in the paragraph above explaining the context. We can just ad the links there instead of confusingly adding them (and then mulitiple sibling links) inside the html form.
Testing
Screen.Recording.2025-07-17.at.21.51.46.mov
Checklist
#ISSUENUM
at the top of your PR to an existing open issue in the mozilla/addons repository.