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

Disclosure Panel and Banner across Automate #5106

Merged
merged 78 commits into from
Jun 18, 2021
Merged

Conversation

SEAjamieD
Copy link
Contributor

@SEAjamieD SEAjamieD commented May 25, 2021

🔩 Description: What code changed, and why?

Some of our customers have been requesting a way to add a disclosure statement to the login page, as well as a ribbon across the top of the login page and Automate in order to notify users of various message that relate to using our product on their network. This update will allow a user to define:

A) if they should show the disclosure text
B) Populate the disclosure with HTML content
C) if they should show the ribbon
D) Populate the ribbon with text, background color, and text color

⛓️ Related Resources

Addresses: #4934 and #4974

👍 Definition of Done

A user can choose to display a banner across the top of the screen in their login flow

  • They may provide their own text in the banner
  • They may choose the background color and text color

A user can choose to display disclosure text on their login page

  • They may populate the disclosure with HTML of their choice

Inside Automate

  • Users may update configuration files so that they can show a customizable banner and disclosure text
  • Unit tests completed that appropriately test the Warning Banner Component
  • Screen shot showing what to expect when banner is present
  • Screen shot showing what to expect when disclosure is present
  • No visual changes to Automate UI if a user choose to not show the banner or the disclosure text.

👟 How to Build and Test the Change

You'll have to rebuild 5 services for this to populate correctly.
The related services are:

  1. automate-deployment
  2. automate-cli
  3. automate-load-balancer
  4. automate-gateway
  5. automate-dex

In hab studio:
build components/automate-deployment
build components/automate-cli
build components/automate-load-balancer
build components/automate-gateway
build components/automate-dex
start_all_services

You'll also need to run chef-automate config patch dev/patch.toml in hab studio. This will connect all the variables we have coming from the config go files.

If you are already logged into Automate UI, please log out and visit https://a2-dev.test/
If all went well, you should see a block of disclosure text below the login, as well as a blue banner across the top of the screen.

You can then toggle the text of these, or show/hide by opening up the patch.toml file and changing the show attributes to false or changing the text. In order to pick up the new changes you'll need to run chef-automate config patch dev/patch.toml again in hab studio.

✅ Checklist

All PRs must tick these:

With occasional exceptions, all PRs from Progress employees must tick these:

  • Is the code clear? (complicated code or lots of comments--subdivide and use well-named methods, meaningful variable names, etc.)
  • Consistency checked? (user notifications, user prompts, visual patterns, code patterns, variable names)
  • Repeated code blocks eliminated? (adapt and reuse existing components, blocks, functions, etc.)
  • Spelling, grammar, typos checked? (at a minimum use make spell in any component directory)
  • Code well-formatted? (indents, line breaks, etc. improve rather than hinder readability)

All PRs from Progress employees should tick these if appropriate:

  • Tests added/updated? (all new code needs new tests)
  • Docs added/updated? (all customer-facing changes)

Please add a note next to any checkbox above if you are NOT ticking it.

📷 Screenshots, if applicable

Banner and Disclosure text in Login
Screen Shot 2021-05-25 at 2 15 43 PM

Banner in Automate
Screen Shot 2021-05-25 at 2 16 07 PM

@SEAjamieD SEAjamieD self-assigned this May 25, 2021
@netlify
Copy link

netlify bot commented May 25, 2021

👷 Deploy Preview for chef-automate processing.

🔨 Explore the source changes: 018d106

🔍 Inspect the deploy log: https://app.netlify.com/sites/chef-automate/deploys/60c21112d79c400007d5c654

@SEAjamieD SEAjamieD added aha-idea automate-ui customer-reported issues reported by customers labels May 25, 2021
This was referenced May 25, 2021
@SEAjamieD SEAjamieD marked this pull request as ready for review May 25, 2021 21:17
</head>

<body>
{{ if eq ("showBanner" | extra) "true" }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In these blocks, we're checking that the user has set their instance to show the banner, if it does, then we go ahead and display the banner. You'll see this similar type of block used both in this file and in password.html. Checking against extra is a pattern that comes from DexIDP (https://dexidp.io/docs/templates/)

<div class="logo">
<img class="theme-navbar__logo" alt="Chef Automate" src="{{url .ReqPath "static/img/automate-blue-d9789f4b.svg" }}">
<img class="theme-navbar__logo" alt="Chef Automate"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'll see a few changes like this as well - there is no change in the outcome. Semgrep was complaining we were not interpolating the template variables properly, so we've updated while we're here.

Comment on lines +55 to +65
<script>
const populateDisclaimer = () => {
const show = '{{ "showDisclosure" | extra }}';
if (show !== 'true') return;

const d = '{{ "disclosureMessage" | extra }}';
document.getElementById('disclosure').innerHTML = d;
}

populateDisclaimer();
</script>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to get true HTML populating the disclosure block, we had to use JavaScript within the same file. In this block, We are getting the disclosure text, and then appending it to the disclosure block.

Comment on lines +362 to +374
// Initilization for warning banner component
{
provide: APP_INITIALIZER,
multi: true,
deps: [AppConfigService],
useFactory: (appConfigService: AppConfigService) => {
return () => {
return appConfigService.loadAppConfig();
};
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What this block does is allow us to call the AppConfigService's loadConfigApp before the application renders. In this case, we are using that time to grab our configuration for the warning banner.

Comment on lines 22 to 34
public loadAppConfig() {
return new HttpClient(this.handler).get('/banner.js')
.toPromise()
.then(data => this.appConfig = data);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the function that is doing all the heavy lifting. It is reaching out to /banner.js and returning our configuration data

Comment on lines +1 to +10
<h1>Warning</h1>
<p><b style="color: blue">Lorem ipsum</b> dolor sit amet, consectetur adipiscing elit. Nulla a diam porttitor, dignissim tortor eu, cursus urna. Aenean sit amet lorem tincidunt, imperdiet lacus eget, finibus sem. Suspendisse quis odio tempor, semper lorem ac, viverra purus. Ut suscipit mollis libero ac venenatis. Nulla facilisi. Nam imperdiet mi magna, eu molestie mauris eleifend quis. Nam placerat mauris massa, vel dapibus sapien venenatis ac. Interdum et malesuada fames ac ante ipsum primis in faucibus. Vestibulum at congue nibh, non egestas tellus.<p>

<p>Nullam et nulla ornare, viverra augue sit amet, malesuada nisl. Mauris nulla lectus, molestie eget auctor id, pharetra eu eros. Proin non nisi varius, convallis libero id, ullamcorper purus. Mauris hendrerit auctor felis non faucibus. Vestibulum mollis ligula odio, eu vestibulum ante ullamcorper sed. In condimentum elit sapien, nec porttitor mi porttitor et. Donec condimentum posuere lorem, molestie ultricies quam eleifend quis. Sed volutpat purus ante, eu tristique lorem efficitur efficitur.</p>

<p>Nunc nibh eros, malesuada posuere molestie nec, pharetra eleifend magna. Nulla mollis vitae dui ac placerat. In nec pulvinar ex. Cras sodales at sapien finibus fermentum. Sed laoreet tempus velit sit amet condimentum. Ut sed risus id justo pulvinar sodales. Nunc eros velit, sollicitudin sit amet posuere vitae, vulputate in nunc. Maecenas quis risus maximus, finibus augue in, faucibus est.<p>

<p>Nam aliquet maximus hendrerit. Duis lacinia faucibus vulputate. Donec id turpis arcu. Nullam pellentesque lobortis justo id lobortis. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Proin in ornare urna. Mauris id arcu enim. In nisi lectus, porta a aliquam in, porta sit amet urna. Suspendisse sollicitudin purus vitae lorem faucibus, a accumsan leo malesuada. Pellentesque egestas vitae nisi eu pulvinar. Proin magna arcu, ullamcorper eget venenatis ut, vehicula ac mi.</p>

<p>Here is a link to <a href="https://google.com" rel="noopener noreferrer" target="_blank">google</a></p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is our demo file for html. Though it is a text file, a user will drop in their desired html for the disclosure panel.

dev/patch.toml Outdated
Comment on lines 1 to 12
[global.v1]
[global.v1.disclosure]
show = true
message_file_path = "/src/dev/banner_message.txt"
[global.v1.banner]
show = true
message = "Changing the text around"
background_color = "3864f2" # Must be a hex value minus the hash symbol
text_color = "FFF" # Must be a hex value minus the hash symbol

# Find valid HEX codes at https://htmlcolorcodes.com/
# check your HTML validation at https://validator.w3.org/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where a user will define their global configurations.

Copy link
Collaborator

@vinay033 vinay033 left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@himanshi-chhabra himanshi-chhabra force-pushed the jamie/banner-combine branch 2 times, most recently from cd09087 to 7d4c4ac Compare June 10, 2021 06:28
seajamied added 2 commits June 10, 2021 18:47
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
kalroy and others added 21 commits June 10, 2021 18:47
Signed-off-by: Kallol Roy <karoy@progress.com>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
Signed-off-by: Himanshi Chhabra <hchhabra@progress.com>
@sonarcloud
Copy link

sonarcloud bot commented Jun 10, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
9.1% 9.1% Duplication

Copy link
Contributor

@chaitali-mane chaitali-mane left a comment

Choose a reason for hiding this comment

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

Changes looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show Warning Banner throughout Automate-UI Display disclosure splash page before login
5 participants