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

Improve popup on special pages #1737

Merged
merged 8 commits into from
Oct 24, 2017
Merged

Conversation

ghostwords
Copy link
Member

@ghostwords ghostwords commented Oct 18, 2017

Fixes #1625.

Fixes #1311, fixes #1492 by virtue of reloading the popup after clicking the undo arrow (as well as the enable/disable Badger buttons). This doesn't need to be in this PR; if it causes trouble, let's break it out.

Related to #1627.

Does not fix:

Before:
screenshot from 2017-10-18 11 21 26

After:
screenshot from 2017-10-19 10 12 13

@ghostwords ghostwords added ui User interface modifications; related to but not the same as the "ux" label ux User experience research needed labels Oct 18, 2017
@ghostwords
Copy link
Member Author

@lemnis @andresbase I have not tested on Android, I hope this doesn't break anything.

@ghostwords
Copy link
Member Author

Need to fix (or disable, maybe until #1634) popup tests. The problem is that the popup is opened in a situation that looks like a special browser page, so that site whitelisting and error reporting buttons as well as the trackers link are now missing. They should be, but the tests still look for them.

@ghostwords
Copy link
Member Author

Tests were also failing since the enable/disable buttons test is no longer applicable as we close the popup instead. I removed it for now (b6b1912).

Pulled in 232c3c0 from #1430 to assist with getting the remaining popup tests passing again.

@ghostwords
Copy link
Member Author

@andresbase Sorry, I added a few potentially breaking changes while fixing tests; we should retest.

ghostwords and others added 5 commits October 18, 2017 16:51
To avoid inconsistent display/various popup bugs.
We now use the same function to build the tabData datastructure
everywhere.

We also build it for all tabs in all windows on boot up. Previously we
were just building it for tabs in the current window. So if you had a
background console open to PB, and refreshed with Ctrl+R, it would not
build the tabData for the tabs in the other windows.

This was causing errors like "Cannot read property "trackers" of
undefined."
Popup now gets closed instead.
@ghostwords ghostwords force-pushed the improve-popup-on-special-pages branch 3 times, most recently from 243eb75 to 03004f0 Compare October 18, 2017 21:32
@ghostwords ghostwords force-pushed the improve-popup-on-special-pages branch 3 times, most recently from 732d9e2 to e4be4d3 Compare October 19, 2017 15:19
This fixes the deactivation button being shown on special browser pages.
@andresbase
Copy link

andresbase commented Oct 20, 2017

Tests in macOS, Windows 10, Linux with Chrome and FF Latest versions:

  • Don't show tracker in options

  • Sliders when reloading to correct position

  • Undo arrow reloads page and closes popup

  • Android

  • Linux FF 56.0 (Some weirdness, need to investigate if it's my install or something else)

@andresbase
Copy link

I was also thinking that the text "There are no third party resources on this site." could be larger or bold so it stands out from all the other text above it that says the usual "Privacy Badger detected 0..."

For the largo logo after inquiring with other people it seems it looks ok while we redesign the UI.

@andresbase
Copy link

The tooltip is empty for firefox
tooltip

Tooltipster: one or more tooltips are already attached to the element below. Ignoring.  tooltipster.bundle.min.js:1:28554
<a id="options" href="/skin/options.html" class="control-button grey-button tooltip tooltipstered" style="display:block" target="_blank">  tooltipster.bundle.min.js:1:28656
Tooltipster: one or more tooltips are already attached to the element below. Ignoring.  tooltipster.bundle.min.js:1:28554
<a id="help" href="/skin/firstRun.html" class="control-button grey-button tooltip tooltipstered" style="display:block" target="_blank">  tooltipster.bundle.min.js:1:28656

@ghostwords
Copy link
Member Author

What are the steps to reproduce?

@andresbase
Copy link

I just installed PB in Firefox 56 and hover over the border when you click on the PB icon.

@ghostwords
Copy link
Member Author

ghostwords commented Oct 20, 2017

I can't reproduce. Could it be that you installed Badger from a branch that doesn't have the new translation strings used in your screenshot, and then switched branches but didn't reload Badger in the browser? Translations are loaded on Badger startup only. A few files (popup.html, popup.js, options.html, options.js) don't seem to require reloading to see new versions of, but most everything else does.

@andresbase
Copy link

Indeed, that must have been the problem, thanks! I can't reproduce the performance issues anywhere else either. They are persistent in my work machine but nowhere else.

@ghostwords ghostwords merged commit a202784 into master Oct 24, 2017
ghostwords added a commit that referenced this pull request Oct 24, 2017
Hide irrelevant text/buttons on special browser pages.

Close popup after doing anything that reloads the page.
@ghostwords ghostwords deleted the improve-popup-on-special-pages branch October 24, 2017 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted ui User interface modifications; related to but not the same as the "ux" label ux User experience research needed
Projects
None yet
3 participants