Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Add "Reset to Defaults" Option in Popup UI, Fix #10004#15411

Merged
Hainish merged 8 commits intoEFForg:masterfrom
cschanaj:refractor-rules-js
May 24, 2018
Merged

Add "Reset to Defaults" Option in Popup UI, Fix #10004#15411
Hainish merged 8 commits intoEFForg:masterfrom
cschanaj:refractor-rules-js

Conversation

@cschanaj
Copy link
Copy Markdown
Collaborator

@cschanaj cschanaj commented May 13, 2018

ping @Hainish

Steps:

  1. Visit https://github.com/
  2. Disable Github, refresh page to make sure that ruleset is disabled
  3. Press "Reset to Defaults"; reload pages and observe that Github is enabled.

BTW, it looks like that Travis selenium tests fail frequently these days, e.g. https://travis-ci.org/EFForg/https-everywhere/jobs/378370329; not sure if #15396 helps, though...

@cschanaj
Copy link
Copy Markdown
Collaborator Author

@Hainish would you have a look on this?

Copy link
Copy Markdown
Member

@Hainish Hainish left a comment

Choose a reason for hiding this comment

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

Thanks for this @cschanaj. A few comments:

  1. I would have the "Reset to Defaults" link appear below the rules sections.
  2. I would have a confirmation dialogue appear. It's too easy to click this accidentally.
  3. I would close the menu once it is clicked. With the current functionality, the page reloads with the popup still open, and you have to close it and open it again to see the changes reflected.

@cschanaj
Copy link
Copy Markdown
Collaborator Author

@Hainish corresponding changes are made. thanks!!

@Hainish Hainish merged commit 63ae754 into EFForg:master May 24, 2018
@Hainish
Copy link
Copy Markdown
Member

Hainish commented May 24, 2018

Looks good, thank you!

@cschanaj cschanaj deleted the refractor-rules-js branch May 25, 2018 11:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants