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

Supporter panels and partial theme handling #154

Merged
merged 66 commits into from Aug 31, 2018
Merged

Conversation

@zarembsky
Copy link
Contributor

@zarembsky zarembsky commented Aug 7, 2018

  • [ x] Have you followed the guidelines in CONTRIBUTING.md?
  • [ x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • [x ] Have you added an explanation of what your changes do?
  • [x ] Does your submission pass tests?
  • [ x] Did you lint your code prior to submission?
zarembsky added 29 commits Jul 17, 2018
# Conflicts:
#	app/panel/components/HeaderMenu.jsx
#	app/panel/constants/constants.js
#	app/panel/reducers/index.js
#	src/classes/ConfData.js
@zarembsky zarembsky requested a review from ghostery/ghostery as a code owner Aug 7, 2018
@zarembsky
Copy link
Contributor Author

@zarembsky zarembsky commented Aug 22, 2018

The last changes were mostly about excluding usage of React Link to get rid of right-click system context menus on kebab menu items, header logo and supporter badge. It is easy to suppress context menu globally, but then it won't show 'Inspect' item, and we better keep it, to convey open source spirit.

jsignanini and others added 5 commits Aug 24, 2018
<path d="M7.136 4.52c.858 0 1.554 1.046 1.554 2.335 0 1.288-.696 2.333-1.554 2.333-.857 0-1.553-1.045-1.553-2.333 0-1.29.696-2.334 1.553-2.334M9.595 13.847c-1.89 0-3.482-1.765-3.96-3.73.925 1.208 2.354 1.985 3.96 1.985 1.605 0 3.035-.777 3.96-1.985-.48 1.965-2.07 3.73-3.96 3.73M12.053 9.188c-.858 0-1.553-1.045-1.553-2.333 0-1.29.695-2.334 1.553-2.334.86 0 1.553 1.046 1.553 2.335 0 1.288-.694 2.333-1.553 2.333" fill="#00AEF0" />
<path d="M27.967 9.838h2.446v3.54c0 1.787-.89 2.808-2.605 2.808-1.716 0-2.605-1.02-2.605-2.807V7.572c0-1.787.89-2.807 2.605-2.807 1.715 0 2.605 1.02 2.605 2.807v1.085H28.76V7.46c0-.796-.348-1.1-.904-1.1-.557 0-.906.304-.906 1.1v6.03c0 .798.35 1.085.906 1.085s.905-.287.905-1.085v-2.057h-.793V9.838M33.462 15.938h-1.81V4.766h1.81v4.788h2.056V4.766h1.842v11.172h-1.842V11.15h-2.056v4.788M38.6 7.573c0-1.786.965-2.807 2.73-2.807 1.765 0 2.73 1.02 2.73 2.807v5.806c0 1.785-.965 2.806-2.73 2.806-1.765 0-2.73-1.02-2.73-2.807V7.572zm1.798 5.917c0 .798.36 1.1.932 1.1.572 0 .93-.302.93-1.1V7.46c0-.796-.358-1.1-.93-1.1-.572 0-.932.304-.932 1.1v6.03zM47.424 4.766c1.7 0 2.574 1.02 2.574 2.808v.35h-1.652v-.462c0-.798-.318-1.1-.875-1.1-.554 0-.872.302-.872 1.1 0 2.296 3.415 2.727 3.415 5.917 0 1.785-.89 2.806-2.605 2.806-1.715 0-2.605-1.02-2.605-2.807v-.687h1.652v.797c0 .798.35 1.085.906 1.085s.906-.287.906-1.085c0-2.297-3.415-2.727-3.415-5.916 0-1.787.874-2.808 2.574-2.808M50.51 4.766h5.458v1.597h-1.846v9.575h-1.766V6.363H50.51V4.766M58.78 9.474h2.497v1.596H58.78v3.27h3.142v1.598H56.96V4.766h4.962v1.596H58.78v3.112M66.585 15.938c-.095-.288-.16-.464-.16-1.373V12.81c0-1.038-.35-1.42-1.15-1.42h-.605v4.548h-1.755V4.766h2.65c1.82 0 2.6.846 2.6 2.57v.878c0 1.148-.366 1.9-1.148 2.265.877.366 1.165 1.212 1.165 2.377v1.724c0 .544.016.943.19 1.358h-1.787zM64.67 6.362v3.43h.687c.654 0 1.052-.286 1.052-1.18v-1.1c0-.8-.27-1.15-.893-1.15h-.847zM70.843 12.235l-2.222-7.47h1.84l1.343 5.092 1.342-5.09h1.68L72.6 12.234v3.703h-1.76v-3.703" fill="#FFF" />
</g>
</svg>

This comment has been minimized.

@jsignanini

jsignanini Aug 27, 2018
Member

@zarembsky can we use ReactSVG here?

This comment has been minimized.

@zarembsky

zarembsky Aug 27, 2018
Author Contributor

We have legacy inlined SVG in many places. They are not affected by skin, so it is sufficient to switch to files and use background-image. I can do it throughout the code if we have time for it.

This comment has been minimized.

@jsignanini

jsignanini Aug 27, 2018
Member

@zarembsky it's fine, just do it here for this ticket. We can take care of the other inlined SVG at a later time.

const rightLink = this.generateLink();
const badgeClasses = `columns shrink ${(!loggedIn || !subscriber) ? 'non-subscriber-badge' : 'subscriber-badge'}`;

This comment has been minimized.

@jsignanini

jsignanini Aug 27, 2018
Member

@zarembsky let's use the classnames library for these dynamic classes

This comment has been minimized.

@zarembsky

zarembsky Aug 27, 2018
Author Contributor

OK

if (userScopes.indexOf('subscription:supporter') >= 0) { return true; }

return false;
}

This comment has been minimized.

@jsignanini

jsignanini Aug 27, 2018
Member

@zarembsky we should be using Account.hasScopesUnverified here.

This comment has been minimized.

@zarembsky

zarembsky Aug 27, 2018
Author Contributor

Account.hasScopesUnverified is a background API, which uses conf.

}
account.getTheme(`${message.currentTheme}.css`).then((theme) => {
if (theme) {
const themes = conf.themes || {};

This comment has been minimized.

@jsignanini

jsignanini Aug 27, 2018
Member

@zarembsky can we set conf.themes to null instead of an empty object when not set?

This comment has been minimized.

@zarembsky

zarembsky Aug 27, 2018
Author Contributor

No, if we look at the next line:
const themes = conf.themes || {};
themes[message.currentTheme] = theme;

zarembsky and others added 4 commits Aug 27, 2018
let currentTheme = this._confData.get('currentTheme');
let theme;
if (currentTheme !== 'default') {
theme = (this._confData.get('themes') || {})[currentTheme];

This comment has been minimized.

@jsignanini

jsignanini Aug 30, 2018
Member

@zarembsky this should now be theme = this._confData.get('themes')[currentTheme]; right?

jsignanini and others added 4 commits Aug 30, 2018
}
}

export default SubscriptionMenu;

This comment has been minimized.

@jsignanini

jsignanini Aug 31, 2018
Member

@zarembsky please rework this component using NavLink instead of Link so that active state navigation is taken care of for us by that element. See the latest changes on this branch to SubscriptionMenu.jsx.

jsignanini added 4 commits Aug 31, 2018
@jsignanini jsignanini merged commit 07a8eb5 into develop Aug 31, 2018
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
@jsignanini jsignanini deleted the feature/supporter branch Aug 31, 2018
jsignanini added a commit that referenced this pull request Nov 20, 2018
* Adding Supporter menu item (GH-1253)

* Implementing conditional enabling/disabling of Supporter menu item.

* Header badge implemented

* Adding Subscription components.

* Adding react files to support Subscription view in the panel.

* Adding Golden Badge panel.

* Moving inline styles for subscriber panel to scss.

* Merge discrepancies

* Working on Themes

* Styling

* Good state #1.

* Adding fields to Subscription info

* Continue styling.

* Adding Subscribe panel.

* Adding support for subscription data in the background.

* Connecting background

* Implementing supporter connection.

* Code cleanup

* Working on Midnight theme

* Working on Midnight theme.

* Adding persisting of selected theme.

* Persisting selected theme.

* Style adjustments.

* Lint and simplification.

* Adjustments according to changed requirements

* More or less stable.

* Multiple fixes.

* Adding tracker stats

* Fix for a bug.

* Commiting lint changes

* Removing inlined svg. Replacing supporter to subscriber.

* Adding svg files for menu item and badge

* Adding react-svg package.

* Adding manage-subscription.svg

* Adding subscribe-badge.svg

* Removing context menu from menu item

* Improving UI.

* Lint fixes.

* Adding $2

* Changing $gold color

* Changing highlight color

* Requested changes.

* Additional requested changes

* Minor fixes.

* Improve setTheme remove logic.

* Fixes for theme.

* Implement NavLink to ensure the correct route is marked as active.

* Replace Link with NavLink.

* Minor fixes.

* More minor fixes.
jsignanini added a commit that referenced this pull request Nov 20, 2018
* Adding Supporter menu item (GH-1253)

* Implementing conditional enabling/disabling of Supporter menu item.

* Header badge implemented

* Adding Subscription components.

* Adding react files to support Subscription view in the panel.

* Adding Golden Badge panel.

* Moving inline styles for subscriber panel to scss.

* Merge discrepancies

* Working on Themes

* Styling

* Good state #1.

* Adding fields to Subscription info

* Continue styling.

* Adding Subscribe panel.

* Adding support for subscription data in the background.

* Connecting background

* Implementing supporter connection.

* Code cleanup

* Working on Midnight theme

* Working on Midnight theme.

* Adding persisting of selected theme.

* Persisting selected theme.

* Style adjustments.

* Lint and simplification.

* Adjustments according to changed requirements

* More or less stable.

* Multiple fixes.

* Adding tracker stats

* Fix for a bug.

* Commiting lint changes

* Removing inlined svg. Replacing supporter to subscriber.

* Adding svg files for menu item and badge

* Adding react-svg package.

* Adding manage-subscription.svg

* Adding subscribe-badge.svg

* Removing context menu from menu item

* Improving UI.

* Lint fixes.

* Adding $2

* Changing $gold color

* Changing highlight color

* Requested changes.

* Additional requested changes

* Minor fixes.

* Improve setTheme remove logic.

* Fixes for theme.

* Implement NavLink to ensure the correct route is marked as active.

* Replace Link with NavLink.

* Minor fixes.

* More minor fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants