Skip to content

/ ghostery-extension

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

Feature/gh 1733/legal updates #414

Merged
merged 17 commits into from Jul 18, 2019
+168 −28
Merged

Feature/gh 1733/legal updates #414

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a2e4762
Begin implementation of legal consent checkbox
wlycdgr Jul 1, 2019
5a1f223
Add Public License and Privacy Policy links
wlycdgr Jul 1, 2019
466dd63
Remove old legal disclaimer from account creation form
wlycdgr Jul 1, 2019
84e44bb
Fix account creation form legal consent checkbox formatting
wlycdgr Jul 1, 2019
c00031f
Merge branch 'develop' into feature/GH-1733/legal-updates
wlycdgr Jul 3, 2019
5276732
Disable linting for legal terms checkbox label because linter is conf…
wlycdgr Jul 3, 2019
cbeafde
Merge branch 'develop' into feature/GH-1733/legal-updates
wlycdgr Jul 3, 2019
2f8662d
Block account creation in extension if legal consent checkbox is unch…
wlycdgr Jul 3, 2019
8ab9c4f
Remove placeholder legal consent string from messages json. Add place…
wlycdgr Jul 17, 2019
7799238
Swap in correct legal consent string for hub account creation form. A…
wlycdgr Jul 17, 2019
01f49c4
Implement legal consent checkbox state change handling
wlycdgr Jul 17, 2019
e8232ac
Improve and tidy legal consent checkbox UI in hub
wlycdgr Jul 17, 2019
da542fa
Update hub account creation form snapshots
wlycdgr Jul 17, 2019
30d725a
Resolve merge conflict in messages. Merge develop in.
wlycdgr Jul 17, 2019
b5a99d3
Reverse order of checkboxes in extension account creation form to mat…
wlycdgr Jul 17, 2019
87279fd
Merge branch 'develop' into feature/GH-1733/legal-updates
wlycdgr Jul 18, 2019
9eaa3ae
Change setState to use callback in hub account creation form checkbox…
wlycdgr Jul 18, 2019
File filter
Filter file types
Clear filters
Jump to
Jump to file
Failed to load files.

Always

Just for now

Prev Next
Loading status checks…

Reverse order of checkboxes in extension account creation form to mat… 

…ch spec
  • Loading branch information
@wlycdgr
wlycdgr committed Jul 17, 2019
commit b5a99d3e9fa6a6643295d1481819d12aaff6176c
14 app/panel/components/CreateAccount.jsx
View file
Open in desktop
@@ -212,18 +212,18 @@ class CreateAccount extends React.Component {
</div>
<div className="row">
<div className="small-12 columns">
<div id="create-account-promotions">
<input id="promotionsChecked" name="promotionsChecked" type="checkbox" checked={promotionsChecked} onChange={this.handleCheckboxChange} />
<label htmlFor="promotionsChecked">{t('hub_create_account_checkbox_promotions')}</label>
<div id="create-account-legal-consent-checkbox" className={(legalConsentNotCheckedError ? 'checkbox-error' : '')}>
<input id="legalConsentChecked" name="legalConsentChecked" type="checkbox" checked={legalConsentChecked} onChange={this.handleCheckboxChange} />
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
This conversation was marked as resolved by christophertino

This comment has been minimized.

Sign in to view
@christophertino

christophertino Jul 17, 2019
Member

Do we still need this lint exclusion?

This comment has been minimized.

Sign in to view
@wlycdgr

wlycdgr Jul 18, 2019 •
edited
Author Member

Without it, the jsx-a11y linter plugin complains that "A form label must be associated with a control". It thinks the label is empty because we are using dangerouslySetInnerHTML attribute to set the text instead of inserting the string directly into the tag as with the promotions checkbox. But if we don't use dangerouslySetInnerHTML, the links in the string are displayed as plain text HTML.

This feels almost like a bug / unaccounted-for use case in the interaction between React and the linter plugin. Given that, I think this is a case where disabling the linter is the lesser of two evils, the other option being changing the checkbox implementation and probably making it more complicated and inconsistent with the promotions checkbox implementation. What do you think?
<label htmlFor="legalConsentChecked" dangerouslySetInnerHTML={{ __html: t('create_account_form_legal_consent_checkbox_label') }} />
</div>
</div>
</div>
<div className="row">
<div className="small-12 columns">
<div id="create-account-legal-consent-checkbox" className={(legalConsentNotCheckedError ? 'checkbox-error' : '')}>
<input id="legalConsentChecked" name="legalConsentChecked" type="checkbox" checked={legalConsentChecked} onChange={this.handleCheckboxChange} />
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
<label htmlFor="legalConsentChecked" dangerouslySetInnerHTML={{ __html: t('create_account_form_legal_consent_checkbox_label') }} />
<div id="create-account-promotions">
<input id="promotionsChecked" name="promotionsChecked" type="checkbox" checked={promotionsChecked} onChange={this.handleCheckboxChange} />
<label htmlFor="promotionsChecked">{t('hub_create_account_checkbox_promotions')}</label>
</div>
</div>
</div>
ProTip! Use n and p to navigate between commits in a pull request.