
Loading…
UI for Fennec (Firefox for Android) #956
- The observer you added to
bootstrap.jsshould go intovapi-backgournd.js(unless you have a good reason to leave it there), also registering and unregistering should go into their own methods on the observer object (consider it as a convention) - vAPI.getURL is unnecessary with vAPI.tabs.open
- Try to follow the project's code styling, also you have some missing semi-colons (jshint)
- I would prefer not to have two large indented block for the toolbarbutton code, so instead of doing that, you could create two separate files, and load the appropriate one into the
vAPIobject withServices.scriptloader.loadSubScript
Since I can't test it, maybe provide a screenshot of how it looks, I'll believe that it works.
And rebase next time, instead of merging the master to your branch, so we don't see the changes that we made, keeps the history cleaner, and it's easier to review.
Thanks for your feedback, I have made the following changes:
- Options observer moved into vapi-background.js
- redundant vAPI.getURL calls for openTab removed
- Semi-colons added after functions
The toolbar button code can't be simply moved into a loadSubScript because it uses variables declared in vapi-background, like cleanupTasks. I have unindented it for cleaner diffing, though.
I couldn't figure out how to get rebasing to work, so it's had to be another merge, sorry about that.
I'll merge it, and I'll fix later what I think should be...
@AlexVallat I wanted to separate the code into files, but then I decided that it doesn't worth it, and I ended up with what it is now. However, I may reorganized more than it needed, so if you have time, please test it if it's still working.
Also, if it does, then it means that we have Fennec support, can I enable it next time on AMO?
So, should I add an official "Fennec support" as part of the new stuff for current dev build?
I've just grabbed the latest code changes now, and it still seems to be working fine. I have been running with (although not with your latest changes, of course) it for day-to-day use for several days now and have not encountered any issues. I would say it is certainly usable enough under Fennec to be worth enabling support for it in AMO, and listing it as a supported platform in the documentation.
The only caveat is that the element picker is only barely usable on touch screens (no hovering, and no Ctrl clicking). Should you ever decide to redesign the element picker, you might like to keep touch screens in mind when doing so.
you might like to keep touch screens in mind when doing so
How usable is dynamic filtering -- if at all? It also needs hovering.
How usable is dynamic filtering -- if at all? It also needs hovering.
Functional, but only if you already know how to use it. Tapping on the third of the box to toggle the state for that filter works, but there's no clue that that's what's expected from you. To be fair, it's pretty un-obvious how to use it on the desktop too, you really need to read the documentation to understand what's going on.
you really need to read the documentation to understand
This is why it's an advanced feature.
- +1 −2 platform/firefox/install.rdf
- +9 −0 platform/firefox/options.xul
- +138 −52 platform/firefox/vapi-background.js
- +12 −0 src/_locales/en/messages.json
- +2 −0 src/dashboard.html
- +2 −0 src/devtools.html
- +11 −4 src/js/messaging.js
- +21 −9 src/js/popup.js
- +2 −1 src/popup.html
- +1 −0 tools/make-firefox.sh
| @@ -0,0 +1,9 @@ | ||
| +<?xml version="1.0" ?> | ||
| +<vbox xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> | ||
| + <setting type="control"> | ||
| + <vbox> | ||
| + <button id="showDashboardButton"/> | ||
| + <button id="showNetworkLogButton"/> | ||
| + </vbox> | ||
| + </setting> | ||
| +</vbox> |




As per gorhill/uBlock#524 (comment)
Now wrapped up in a pull request for convenience.