Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

noscript info dialog improvements #7434

Merged
merged 1 commit into from
Mar 2, 2017
Merged

noscript info dialog improvements #7434

merged 1 commit into from
Mar 2, 2017

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Mar 1, 2017

Test Plan:

  • go to vox.com
  • disable scripts in the brave panel
  • hit the noscript icon, uncheck everything except vox and vox-cdn checkboxes. click inside the dialog and the dialog should not disappear.
  • hit one of the allow buttons. the page should reload, and vox images should appear.

changes:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

* make it easier to click the checkboxes. fix #7430.
* do not close the dialog unless clicking outside or on a button.
* make the dialog the same whether noscript is globally enabled or not.
* if no checkboxes are checked, clicking the button should be a no-op.
@alexwykoff
Copy link
Contributor

testing now

@alexwykoff
Copy link
Contributor

default allow all on open is perhaps a little odd, but otherwise looks good to me.

@alexwykoff
Copy link
Contributor

@diracdeltas are there any tests which cover this feature?

@diracdeltas
Copy link
Member Author

@alexwykoff yes, the noscript test

@bbondy
Copy link
Member

bbondy commented Mar 2, 2017

++

@bbondy bbondy merged commit 0e68342 into master Mar 2, 2017
@diracdeltas diracdeltas deleted the fix/7430 branch March 26, 2017 12:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hit boxes on allow scripts are too small, create unintended modal closure when missed.
4 participants