Skip to content

/ ghostery-extension

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

Jump to bottom

Gh 1593 and gh 1594/upgrade plus banner and logo #373

Merged
merged 51 commits into from Apr 26, 2019
Merged

Gh 1593 and gh 1594/upgrade plus banner and logo #373

merged 51 commits into from Apr 26, 2019

Conversation

@wlycdgr
Copy link
Member

@wlycdgr wlycdgr commented Apr 17, 2019

Implement new upgrade banner and subscriber badge logolinks on simple and detailed summary view and in header when in expert expanded view.

  • Have you followed the guidelines in CONTRIBUTING.md?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do?
  • Does your submission pass tests?
  • Did you lint your code prior to submission?
wlycdgr added 30 commits Mar 20, 2019
@wlycdgr
Remove old Plus banner/logo
Loading status checks…
b64fdae
@wlycdgr
Merge branch 'develop' into GH-1593_and_GH-1594/UpgradePlus-banner-an…
68378c6 
…d-logo
@wlycdgr
Add green upgrade banner to Summary component
Loading status checks…
a131dfb
@wlycdgr
Merge branch 'develop' into GH-1593_and_GH-1594/UpgradePlus-banner-an…
26953b0 
…d-logo
@wlycdgr
Add upgrade banner class
8e409d6
@wlycdgr
Fix classNames -> className typo
a8f4602
@wlycdgr
Implement CSS for green upgrade banner
cd6148a
@wlycdgr
Display smaller upgrade banner in summary view when on detailed tab
208fcd4
@wlycdgr
Adjust position of smaller upgrade banner upwards a little to avoid o…
dd1de6f 
…verlapping the donut
@wlycdgr
Add gold subscriber icon to display when a subscribed user is signed in
ec59226
@wlycdgr
Finish implementation of plus upgrade banner and gold icon display lo…
b3a380b 
…gic for the case when the panel is not both in detailed view and expanded
@wlycdgr
Remove debug code and redundant variable from Summary#render
4e0af1e
@wlycdgr
Refactor Summary#render for clarity
901e06e
@wlycdgr
Add green upgrade banner and gold plus icon SVGs for expanded view
5c861d9
@wlycdgr
Add expanded view upgrade banner div stub to Header#render. Refactor …
Loading status checks…
d4dedb3 
…and clean up Header#render a bit.
@wlycdgr
Merge branch 'develop' into GH-1593_and_GH-1594/UpgradePlus-banner-an…
Loading status checks…
c9eb688 
…d-logo
@wlycdgr
Display gold icon in expanded view if user is a logged-in subscriber
499051f
@wlycdgr
Stub click on badge in expanded view
6273e6e
@wlycdgr
Replicate dev behavior of badge clicking in Header in expanded view
8c8b7fd
@wlycdgr
Rename upgrade banner / gold plus badge click handler for clarity
Loading status checks…
8e7abf1
@wlycdgr
Add badge CSS classes to extended view badges. These need updating
dbdea1e
@wlycdgr
Correct subscriber redirect path for gold plus icon click handler in …
9aedc21 
…Header
@wlycdgr
Update badge CSS
43b5e81
@wlycdgr
Tweak CSS for expanded view upgrade banner and gold plus icon to spec
Loading status checks…
4cc227b
@wlycdgr
Remove reference to non-extant state.header from header container map…
Loading status checks…
8c4a316 
…StateToProps. pass state.panel.is_expanded to Header
@wlycdgr
Implement show/hide logic for header upgrade banner and gold plus icon
Loading status checks…
740f61b
@wlycdgr
Factor kebab div out to const for clarity in Header#render
Loading status checks…
5ebbd1e
@wlycdgr
Tweak gold plus icon expanded view SVG to make sure Plus is more cent…
Loading status checks…
d0c3b4f 
…ered, with Vinnie's approval. Refactor Header#render for clarity
@wlycdgr
Finish clarity refactor of Summary#render
Loading status checks…
6035749
@wlycdgr
Nudge summary content down in expert view so green upgrade banner can…
Loading status checks…
a0170de 
… fit. Delete two unused scss partials
wlycdgr added 2 commits Apr 18, 2019
@wlycdgr
Remove duplication from return of Summary#render
be447e7
@wlycdgr
Merge branch 'GH-1593_and_GH-1594/UpgradePlus-banner-and-logo' of git…
Loading status checks…
069fca6 
…hub.com:ghostery/ghostery-extension into GH-1593_and_GH-1594/UpgradePlus-banner-and-logo
@IAmThePan
IAmThePan suggested changes Apr 18, 2019
View changes
Copy link
Contributor

@IAmThePan IAmThePan left a comment •
edited

Some Questions:

  1. What is the plan for translating the "Upgrade to Plus" text within the Green Flag which is an SVG?
  2. When I am using the Midnight Theme the Plus Sticker is blue and is missing its lettering in the header and it is gold and unchanged in the Simple & Detailed Views.
  3. The Summary Panel within the Detailed View doesn't extend to the top when I have the Midnight Theme activated.
@wlycdgr
Fix Summary view display in Midnight theme on Detailed tab. Realign g…
Loading status checks…
c38b4b5 
…reen upgrade banner and gold icon accordingly. Add debug and log truebools back into manifest
@wlycdgr
Copy link
Member Author

@wlycdgr wlycdgr commented Apr 18, 2019 •
edited

3 is fixed.
@wlycdgr
Display subscriber badge in header in gold in Midnight theme
Loading status checks…
ce7158b
@wlycdgr
Copy link
Member Author

@wlycdgr wlycdgr commented Apr 19, 2019

2 is fixed.
wlycdgr added 4 commits Apr 19, 2019
@wlycdgr
Add upgrade banner text to locale strings file. Remove SVG upgarde te…
Loading status checks…
80a1736 
…xt. Replace it with a text span.
@wlycdgr
Finalize CSS for upgrade banner text. Delete duplicate upgrade banner…
Loading status checks…
26c6ab0 
… SVG
@wlycdgr
Convert 'upgrade to' text in the green banner from SVG to text for sm…
Loading status checks…
de6aa6b 
…aller detail view banner
@wlycdgr
Make sure upgrade banner or subscriber badge display in the hamburger…
Loading status checks…
c7a43ee 
… dropdown views
@wlycdgr
Copy link
Member Author

@wlycdgr wlycdgr commented Apr 19, 2019

1 is fixed.

@IAmThePan I've addressed all the issues you noted. Also, I've made it so that the banner/badge is visible in the various hamburger views. This is ready for re-review.
@wlycdgr wlycdgr added the v8.4.0 label Apr 19, 2019
@IAmThePan
Merge branch 'develop' into GH-1593_and_GH-1594/UpgradePlus-banner-an…
Verified
This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: 4AEE18F83AFDEB23 Learn about vigilant mode.
Loading status checks…
ed9f928 
…d-logo
@IAmThePan
IAmThePan suggested changes Apr 23, 2019
View changes
Copy link
Contributor

@IAmThePan IAmThePan left a comment

It looks pretty good. One thing...

When the language isn't English, the Upgrade To text may be too long/short for the space provided in the green banner. Check out this Leeted text:
Screen Shot 2019-04-23 at 00 46 31
wlycdgr added 3 commits Apr 23, 2019
@wlycdgr
Modify upgrade banner to scale along with text that may vary in lengt…
Loading status checks…
9aef19d 
…h by locale
@wlycdgr
Tweak scalable CSS for simple view upgrade banner
Loading status checks…
4006174
@wlycdgr
Dedupe upgrade banner render code. Clean up CSS. Make sure banners st…
43b5988 
…retch in both detailed and summary views.
@wlycdgr
Copy link
Member Author

@wlycdgr wlycdgr commented Apr 24, 2019

The green upgrade banner implementation has been modified to better handle 'Upgrade To' translations of variable length. Ready for re-review.
@wlycdgr
Merge in develop & resolve conflict in Summary.jsx
Loading status checks…
2affa53
@IAmThePan
IAmThePan approved these changes Apr 24, 2019
View changes
@IAmThePan
IAmThePan suggested changes Apr 24, 2019
View changes
Copy link
Contributor

@IAmThePan IAmThePan left a comment

I'm seeing that the Plus Sticker (signed in, subscriber) isn't in the right position in the Simple an Detailed views.
Screen Shot 2019-04-24 at 13 13 22
wlycdgr added 2 commits Apr 24, 2019
@wlycdgr
Restore mistakenly deleted gold Plus icon CSS
Loading status checks…
56e607a
@wlycdgr
Fix Summary view alignment in Condensed mode
Loading status checks…
bbae8e7
@wlycdgr
Copy link
Member Author

@wlycdgr wlycdgr commented Apr 24, 2019

Fixed
@IAmThePan
IAmThePan approved these changes Apr 24, 2019
View changes
@IAmThePan
Merge branch 'develop' into GH-1593_and_GH-1594/UpgradePlus-banner-an…
Verified
This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: 4AEE18F83AFDEB23 Learn about vigilant mode.
Loading status checks…
88759ba 
…d-logo
@IAmThePan IAmThePan merged commit 3e794d0 into develop Apr 26, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@IAmThePan IAmThePan deleted the GH-1593_and_GH-1594/UpgradePlus-banner-and-logo branch Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IAmThePan IAmThePan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@wlycdgr @IAmThePan