Add Cost Functionality to Goldilocks #524

rbren · 2022-09-26T14:35:09Z

This PR adds cost functionality to goldilocks

Checklist

I have signed the CLA
I have updated/added any relevant documentation

Description

What's the goal of this PR?

Add the ability to show workload costs and cost changes

What changes did you make?

Lots of frontend updates, as well as some backend changes

What alternative solution should we consider, if any?

* developed the UI for the email-box * added validation and query params * removed CSS comments, unusded class attribute, and changed the scripts order * updated css for the email input * removed the validator library * changed div to aside and used the form tag * used accent-color, added label * removed outline:none * updated the outline css for the email input * changed h2 to p * make the privacy policy text as a link * updated the background color for the submit button * updated the text color for the email label * updated the accent-color for the checkbox * updated the text color the actived button and its outline * moved the link outside the label * remove toggleSubmitBtn logic and updated css * moved the link outside label * added required for the checkbox * udpdated the text color for the privacy link and added the underline * used the visually-hidden class

* developed the UI for the email-box * added validation and query params * removed CSS comments, unusded class attribute, and changed the scripts order * updated css for the email input * removed the validator library * changed div to aside and used the form tag * used accent-color, added label * removed outline:none * updated the outline css for the email input * changed h2 to p * make the privacy policy text as a link * updated the background color for the submit button * updated the text color for the email label * updated the accent-color for the checkbox * updated the text color the actived button and its outline * moved the link outside the label * remove toggleSubmitBtn logic and updated css * moved the link outside label * added required for the checkbox * udpdated the text color for the privacy link and added the underline * integrated with the api to get the API token * chnaged insightshost to insights-host * used the visually-hidden class * updated the usage message for the insights-host flag

* updated the footer * updated the font size for the email box toggle title'

* developed the UI for the email-box * added validation and query params * removed CSS comments, unusded class attribute, and changed the scripts order * updated css for the email input * removed the validator library * changed div to aside and used the form tag * used accent-color, added label * removed outline:none * updated the outline css for the email input * changed h2 to p * make the privacy policy text as a link * updated the background color for the submit button * updated the text color for the email label * updated the accent-color for the checkbox * updated the text color the actived button and its outline * moved the link outside the label * remove toggleSubmitBtn logic and updated css * moved the link outside label * added required for the checkbox * udpdated the text color for the privacy link and added the underline * integrated with the api to get the API token * chnaged insightshost to insights-host * developed the api token section * changed the default value for insights-host * updated the text, added the disable cost settings button * add css to disable the buttons when the forms are invalid

* developed the UI for the email-box * added validation and query params * removed CSS comments, unusded class attribute, and changed the scripts order * updated css for the email input * removed the validator library * changed div to aside and used the form tag * used accent-color, added label * removed outline:none * updated the outline css for the email input * changed h2 to p * make the privacy policy text as a link * updated the background color for the submit button * updated the text color for the email label * updated the accent-color for the checkbox * updated the text color the actived button and its outline * moved the link outside the label * remove toggleSubmitBtn logic and updated css * moved the link outside label * added required for the checkbox * udpdated the text color for the privacy link and added the underline * integrated with the api to get the API token * chnaged insightshost to insights-host * developed the api token section * changed the default value for insights-host * updated the text, added the disable cost settings button * add css to disable the buttons when the forms are invalid * developed the cost settings section * updated dashboard.go * refactored the code * refactored the code * updated the initQueryParams function * updated the code based on feedback * shown invalid api token message and updated the step for the input element * updated the step for the input element

* developed the UI for the email-box * added validation and query params * removed CSS comments, unusded class attribute, and changed the scripts order * updated css for the email input * removed the validator library * changed div to aside and used the form tag * used accent-color, added label * removed outline:none * updated the outline css for the email input * changed h2 to p * make the privacy policy text as a link * updated the background color for the submit button * updated the text color for the email label * updated the accent-color for the checkbox * updated the text color the actived button and its outline * moved the link outside the label * remove toggleSubmitBtn logic and updated css * moved the link outside label * added required for the checkbox * udpdated the text color for the privacy link and added the underline * integrated with the api to get the API token * chnaged insightshost to insights-host * developed the api token section * changed the default value for insights-host * updated the text, added the disable cost settings button * add css to disable the buttons when the forms are invalid * developed the cost settings section * updated dashboard.go * refactored the code * refactored the code * updated the initQueryParams function * updated the code based on feedback * shown invalid api token message and updated the step for the input element * updated the step for the input element * calculated the cost * refactored the code based on feedback * refactored the code * updated the code based on feedback * updated the code based on feedback * updated the code based on feedback * updated calculating the container cost * transformed the recommended cost * sorted the instance types, created the default values * updated the code based on feedback * updated label name * fix CPU values Co-authored-by: Robert Brennan <contact@rbren.io>

fairwinds-insights · 2022-09-26T14:35:17Z

Fairwinds Insights CI Report

View the Full Report

✅ No new Action Items detected!

pkg/dashboard/templates/cost_settings.gohtml

sudermanjr · 2022-09-27T16:21:56Z

Can you please fill out the PR description?

Also, has this been tested with a screen reader for accessibility?
There's some JavaScript that's showing/hiding things that make me worry about focus management, as well as the some of the CSS that may cause issues with accessibility.

sudermanjr

Please remove the banner ad for the benchmark report

sudermanjr

Once the cost data is added, every time I click on a namespace, the screen flashes several times as the javascript hides/shows elements. Is it possible to smooth this out?

sudermanjr · 2022-09-27T16:43:38Z

pkg/dashboard/templates/cost_settings.gohtml

+          <label for="cloud-providers">Cloud Provider</label>
+          <div class="cost-settings-box__select-container">
+            <select name="cloud-providers" id="cost-settings-box__cloud-providers">


This label doesn't actually relate to the selector. You can see this in the accessibility section of the dev-tools inspector

sudermanjr · 2022-09-27T16:44:41Z

pkg/dashboard/templates/cost_settings.gohtml

+          <div class="cost-settings-box__select-container">
+            <select name="instance-types" id="cost-settings-box__instance-types">
+            </select> 
+            <img src="/static/images/triangle.svg" alt="Select Icon" />


Since this is purely a visual enhancement, and isn't needed for usability, should it be hidden from screen readers?

(this applies to the other box as well)

rbren · 2022-09-27T16:54:45Z

@hieptl when you have some time, can you address Andy's feedback?

sudermanjr

When I disable javascript, we still see all the forms, but they are un-usable and have no functionality. We should set this so that all the forms are hidden by default, and javascript enables them. This will greatly improve the usability if javascript isn't working or enabled.

rbren · 2022-09-27T16:56:02Z

Re: accessibility, I will see if I can enlist Ivan to help us surface any issues.

sudermanjr · 2022-09-27T17:02:21Z

pkg/dashboard/templates/dashboard.gohtml

+  <script defer src="static/js/email.js"></script>
+  <script defer src="static/js/api-token.js"></script>
+  <script defer src="static/js/cost_settings.js"></script>


Putting these here is an outdated pattern that may affect performance negatively on "large sites" which may be triggered when we have a large amount of data.

An old-fashioned solution to this problem used to be to put your script element right at the bottom of the body (e.g. just before the tag), so that it would load after all the HTML has been parsed. The problem with this solution is that loading/parsing of the script is completely blocked until the HTML DOM has been loaded. On larger sites with lots of JavaScript, this can cause a major performance issue, slowing down your site.

From: https://developer.mozilla.org/en-US/docs/Learn/JavaScript/First_steps/What_is_JavaScript#script_loading_strategies

sudermanjr · 2022-09-27T21:39:11Z

pkg/dashboard/dashboard.go

+	cpuRequests = 0
+	memRequests = 0
+	cpuLimits = 0
+	memLimits = 0


I believe the zero-value of a float in go is zero, and it's not nillable, so these are unnecessary after declaring the vars.

https://go.dev/play/p/98h2Rxl4FAy

sudermanjr · 2022-09-27T21:51:17Z

pkg/dashboard/templates/cost_settings.gohtml

+          </div>
+        </div>
+        <div class="cost-settings-box__instance-types">
+          <label for="instance-types">Instance Type</label>


Same with this label, it doesn't select the cost settings box, so the screen reader never reads it.

sudermanjr · 2022-09-27T22:01:05Z

pkg/dashboard/templates/cost_settings.gohtml

+          ><i aria-hidden="true" class="fas fa-fw fa-angle-up --showWhenOpen"></i>
+        </p>
+        <div>
+         <button id="cost-settings__disable-cost-settings">Disable Cost Settings</button>


This button living inside of the summary element isn't semantic HTML because it puts a button inside of a button. The "Disable Cost Settings" button needs to be outside of this summary element.

You can notice the negative effects of this both visually when you go to click the button and it looks like you're collapsing the form, as well as when you use a screenreader and it reads "Cost settings - disable cost settings" all at once.

There's another instance of this in another form as well.

hieptl · 2022-09-28T18:33:31Z

Hi @sudermanjr : I updated the code based on your feedback. Thank you for your feedback!

sudermanjr

Thanks for all the updates!

hieptl and others added 9 commits August 17, 2022 21:53

Merge branch 'master' into hl/self-identification

4e2a28e

Merge branch 'master' into hl/self-identification

373be0f

Update the Footer (#502)

e5f0bb5

* updated the footer * updated the font size for the email box toggle title'

improved reloading the page (#521)

c312654

rbren requested review from sudermanjr and transient1 as code owners September 26, 2022 14:35

Merge branch 'master' into hl/self-identification

8b0d903

transient1 approved these changes Sep 26, 2022

View reviewed changes

rbren commented Sep 26, 2022

View reviewed changes

pkg/dashboard/templates/cost_settings.gohtml Outdated Show resolved Hide resolved

pkg/dashboard/templates/cost_settings.gohtml Outdated Show resolved Hide resolved

rbren added 3 commits September 26, 2022 13:38

Update pkg/dashboard/templates/cost_settings.gohtml

a20dc53

Update pkg/dashboard/templates/cost_settings.gohtml

20b6711

Merge branch 'master' into hl/self-identification

38296ea

sudermanjr requested changes Sep 27, 2022

View reviewed changes

sudermanjr reviewed Sep 27, 2022

View reviewed changes

sudermanjr requested changes Sep 27, 2022

View reviewed changes

sudermanjr reviewed Sep 27, 2022

View reviewed changes

updated the code based on feedback

fb8af7d

sudermanjr approved these changes Sep 28, 2022

View reviewed changes

sudermanjr merged commit cb026e7 into master Sep 28, 2022

sudermanjr deleted the hl/self-identification branch September 28, 2022 20:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Cost Functionality to Goldilocks #524

Add Cost Functionality to Goldilocks #524

rbren commented Sep 26, 2022 •

edited

Loading

fairwinds-insights bot commented Sep 26, 2022

sudermanjr commented Sep 27, 2022

sudermanjr left a comment

sudermanjr left a comment

sudermanjr Sep 27, 2022

sudermanjr Sep 27, 2022

sudermanjr Sep 27, 2022

rbren commented Sep 27, 2022 •

edited

Loading

sudermanjr left a comment

rbren commented Sep 27, 2022

sudermanjr Sep 27, 2022

sudermanjr Sep 27, 2022 •

edited

Loading

sudermanjr Sep 27, 2022

sudermanjr Sep 27, 2022 •

edited

Loading

hieptl commented Sep 28, 2022

sudermanjr left a comment

Add Cost Functionality to Goldilocks #524

Add Cost Functionality to Goldilocks #524

Conversation

rbren commented Sep 26, 2022 • edited Loading

Checklist

Description

What's the goal of this PR?

What changes did you make?

What alternative solution should we consider, if any?

fairwinds-insights bot commented Sep 26, 2022

Fairwinds Insights CI Report

sudermanjr commented Sep 27, 2022

sudermanjr left a comment

Choose a reason for hiding this comment

sudermanjr left a comment

Choose a reason for hiding this comment

sudermanjr Sep 27, 2022

Choose a reason for hiding this comment

sudermanjr Sep 27, 2022

Choose a reason for hiding this comment

sudermanjr Sep 27, 2022

Choose a reason for hiding this comment

rbren commented Sep 27, 2022 • edited Loading

sudermanjr left a comment

Choose a reason for hiding this comment

rbren commented Sep 27, 2022

sudermanjr Sep 27, 2022

Choose a reason for hiding this comment

sudermanjr Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

sudermanjr Sep 27, 2022

Choose a reason for hiding this comment

sudermanjr Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

hieptl commented Sep 28, 2022

sudermanjr left a comment

Choose a reason for hiding this comment

rbren commented Sep 26, 2022 •

edited

Loading

rbren commented Sep 27, 2022 •

edited

Loading

sudermanjr Sep 27, 2022 •

edited

Loading

sudermanjr Sep 27, 2022 •

edited

Loading