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

Ability to Brand Side Nav and Accordion #167

Merged
merged 7 commits into from
Oct 8, 2019
Merged

Ability to Brand Side Nav and Accordion #167

merged 7 commits into from
Oct 8, 2019

Conversation

StevenUlmer
Copy link
Contributor

closes #158

This adds the ability to brand a couple items in goponents. For now this just includes the 'active' rectangle for both the side nav and the accordion panels. This also removes some unused css from the go-card where branding was being used.

All it takes to brand everything is to import the GoBrandingService into your app.component.ts file and set the brand color through goBrandingService.setBrandColor(color)

@StevenUlmer StevenUlmer added the feature New feature request label Sep 9, 2019
Copy link
Contributor

@deotte deotte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My name is Otter and I approve of this PR. @AlexOverbeck @grahamhency

deotte
deotte previously approved these changes Sep 10, 2019
Copy link
Contributor

@AlexOverbeck AlexOverbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to do this without querying the dom or using ViewChildren. We should also think about making this some sort of observable where we subscribe to the brand data.

@grahamhency grahamhency added this to the v1.3.0 milestone Sep 13, 2019
@StevenUlmer StevenUlmer force-pushed the branding branch 2 times, most recently from 0283fac to 1d9c1a5 Compare September 27, 2019 13:39
@grahamhency grahamhency changed the base branch from master to v1.3.0 October 2, 2019 20:36
Copy link
Contributor

@grahamhency grahamhency left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StevenUlmer all of this looks pretty good. What do you think about switching the naming of this to ConfigService? We can then make a GoConfig interface that has various configurations on it. The two I'm thinking right now would be:

  • brandColor (which you already have)
  • theme

This way, we could expand upon the interface in the future without having to introduce new services for each thing.

@grahamhency
Copy link
Contributor

@StevenUlmer just a couple of other comments on this one.

  1. There are some merge conflicts now
  2. We will need to add some documentation around this. I am okay if you want to open a follow up PR for this after this is merged, but we'll want it included in v1.3.0

Copy link
Contributor

@grahamhency grahamhency left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StevenUlmer just a couple of comments, getting close!

Copy link
Contributor

@grahamhency grahamhency left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boom, this is ready to go! 👍

@StevenUlmer StevenUlmer merged commit 3e96520 into v1.3.0 Oct 8, 2019
@StevenUlmer StevenUlmer deleted the branding branch October 8, 2019 18:26
@grahamhency grahamhency mentioned this pull request Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants