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.

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

Added url blacklist to settings panel #27

Closed
wants to merge 2 commits into from

Conversation

ChrisLTD
Copy link
Contributor

@ChrisLTD ChrisLTD commented Nov 7, 2013

Uses modified code from the Vimium plugin. I had to write it using commas as separators since we only get a one line text box for Safari extensions. It does support * as wildcards.

The input box is a bit small on my screen, so I suggest dropping the "(blank for none)" part of the first setting's title:
screen shot 2013-11-07 at 5 25 13 pm

We could include that information in the readme file instead.

@guyht
Copy link
Collaborator

guyht commented Nov 9, 2013

One issue I have just realised with this is that it won't play nice with tab back and tab forward as when a tab containing a blacklisted URL comes up, the plugin will be disabled.

I think a lot of people use vimari to cycle through tabs and this would break that functionality.

Going to leave this out of the release for the moment while we think of a solution.

@ChrisLTD
Copy link
Contributor Author

ChrisLTD commented Nov 9, 2013

Instead of never letting the keys get bound, we could move the URL check into the keypress event itself?

@ChrisLTD
Copy link
Contributor Author

I took a look at how Vimium and VimFX handle blacklists, and it appears they both completely stop reacting to keyboard events on blacklisted sites, even tab switching commands are ignored.

Perhaps we just note that the blacklist will also stop tab switching in the documentation?

@genericsteele
Copy link

Coming from Vimium in Chrome, I understand this behavior and know what I'm getting into when I blacklist a site. I think it's a reasonable expectation and trade-off.

@guyht
Copy link
Collaborator

guyht commented Nov 13, 2013

Ok. I will merge this over the weekend. If there is a backlash, we can always remove it.

@genericsteele
Copy link

That's awesome. Thank you sir. It's definitely an opt-in behavior, so I'd imagine it'll go pretty smoothly.

@ChrisLTD
Copy link
Contributor Author

You may want to change the defaults I have in the options. I'll bet a lot of people might confused why their tab switching doesn't work on gmail.

@guyht
Copy link
Collaborator

guyht commented Nov 13, 2013

I dont think there should be any 'default' sites. As you say, this can just lead to confusion when it stops working for people on certain pages.

@genericsteele
Copy link

Yeah I agree. Default sites could be a bit of a surprise.

@guyht
Copy link
Collaborator

guyht commented Nov 24, 2013

Merged

@guyht guyht closed this Nov 24, 2013
@guyht
Copy link
Collaborator

guyht commented Nov 24, 2013

Added in v1.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants