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.

Sign up for GitHub

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

Remove permissions and tweak the settings panel #24

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Remove permissions and tweak the settings panel #24

wants to merge 7 commits into from

Conversation

thatlittlegit
Copy link

@thatlittlegit thatlittlegit commented Apr 9, 2021 •
edited
Loading

This PR has a few parts:

  • Remove the <all_urls> permission; this isn't needed, since scripts are only run by Monastery on pages it controls.
  • Parse new URLs that are put into a site list, and extract the hostname and pathname. This (maybe?) fixes Adding URLs to blacklist with https doesn't actually blacklist the site #21.
  • Modify the settings page design to be responsive. I haven't tried it, but this might fix Settings page is wonky in new version with a full screen view #13, since it definitely works with a full browser window and Firefox's settings panel.
  • Keep track of the current delay in seconds in the settings panel. In my opinion, it's more intuitive instead of 0.5 minutes. Plus, something like 7-10 seconds is fine for me, 30s is overkill; now all of those are options!
  • Use native fonts in the settings panel instead of Fira Sans and Open Sans. This makes them load faster and avoid calls to Google's servers.
  • Make removing notifications easier, by putting the remove button to the right of the entry.
  • Move around a few of the elements in the settings panel, see the screenshot below.

Most of these are visible in the following screenshot (Firefox 87 on GNOME, Debian unstable):
Screenshot of the settings panel with the changes from this pull request applied

If you don't like some of the changes, I can revert them (e.g. if you just want the permission change, I can remove most of the others, since the permission change doesn't fit in with everything else).

thatlittlegit added 7 commits April 5, 2021 13:32
Remove the <all_urls> permission
a445288 
Interestingly, it seems that it isn't even needed. The only case where
privileged functionality is needed is tab.executeScript, and that's
only done on the delay page, which is in Monastery's host and
thus (from FF57 on) automatically allowed.

In background.js, we clarify to only notify us if it's the right
page. This prevents errors about not having permission, since we'll be
told multiple times beforehand about the previous page; saying we only
want ours means that we can't possibly error (hopefully).
Keep track of the current delay in seconds
c449f6a 
I find for myself that just seeing the yellow status-page makes me
Ctrl-W very quickly, and that thirty seconds is overkill for stopping
me from procrastinating.

This commit allows configuring in five-second intervals with a minimum
of five seconds.
Adjust style and elements of the settings page
5163523 
This is a bunch of small, separate changes put into one commit. The
main ones:

- Make #delaySwitch/#delayLength refer to the checkbox, not the box
  it's in. Unfortunately, the box still needs to be referred to for CSS
  reasons, so it is called #delaySwitchSection/etc. instead.
- Move the 'remove' button next to the notification, so they can be
  removed one-at-a-time.
- Change the 'Add' button in the notifications section to 'Add
  Notification', to clarify what's being added.
- Remove the 'Notify me when there remains' label: I think it's
  implied by being directly underneath the checkbox.
- Remove many <br/>s to make it more responsive; also adjust the
  flexboxes to meet this goal.
- Put the 'Add' button inline with the sitelist addition entry.
- Change italics in the case-sensitivity warning.
- If there are no notifications, add a notice to clarify.

The only major problems I have at this point are:

- The CSS is very duplicated, it'd be nice to reuse some of it.
- querySelector is used a lot in the code, it'd be nice to try and
  avoid using it so much.
- The 'Remove' button for a sitelist is very close to the <select/>;
  unfortunately, I don't have a better way to do the removal with
  <select/>, and implementing a custom one doesn't seem like a good
  idea.
Use the native fonts instead of Fira Sans/Open Sans
de0a34d 
Fira Sans looks nice, but it isn't native and results in a request to
Google instead of just using the built-in font. The settings panel
will automatically use a good font.

Unfortunately, the popup won't, and will default to `serif`. In
addition, Firefox doesn't support system-ui, so that can't be used
either. Right now, I put it to system-ui, sans-serif so it will use it
when it can, but unfortunately it will be the slightly-uglier
sans-serif right now. I do think that this is better than fetching it
from Google Fonts, though.
Tweak "Blocking Settings" settings widgets
ffb4179 
This was primarily done so the 'Save' button wouldn't block the
"Notification Settings" header, but it also let me tweak some other
details. For example, the input boxes are now smaller, and the save
button is naturally sized.
Ensure that the sitelist addition input won't be too big
a28677f
Extract the hostname and path from a new valid URL
b856872 
Fixes omivore#21. Tries to parse the input as a URL. If it fails, then oh
well; otherwise, it will extract the hostname, port and pathname and
put it in automatically. It even recognizes duplicates: if
'google.com' is already in there, then typing 'https://google.com'
will be caught as a duplicate!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@thatlittlegit