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

Fixed some issues with dashboard #570

Merged
merged 1 commit into from Jan 25, 2021

Conversation

bexsoft
Copy link
Collaborator

@bexsoft bexsoft commented Jan 21, 2021

What does this do?

Fixed some issues with dashboard UX

-Added padding to the bottom of dashboard

-Added calculations for linear chart tick interval

-Added default min width configurations to panels.

-Fixed potential crash with clean tenants

How does it look?

Screen Shot 2021-01-21 at 16 59 02

Screen Shot 2021-01-21 at 16 58 44

Screen Shot 2021-01-21 at 16 58 31

@bexsoft bexsoft added the UI User Interface label Jan 21, 2021
@bexsoft bexsoft self-assigned this Jan 21, 2021
@Alevsk
Copy link
Contributor

Alevsk commented Jan 21, 2021

LGTM, it just have some compilation warnings

Compiled with warnings.

./src/screens/Console/Dashboard/Prometheus/PrDashboard.tsx
  Line 17:28:  'useMemo' is defined but never used                                                                                  @typescript-eslint/no-unused-vars
  Line 159:5:  React Hook useCallback has a missing dependency: 'dashboardDistr'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

./src/screens/Console/Dashboard/Prometheus/Widgets/LinearGraphWidget.tsx
  Line 81:18:  Expected '!==' and instead saw '!='  eqeqeq

Alevsk
Alevsk previously approved these changes Jan 21, 2021
return JSON.parse(atob(storedConfiguration));
const parsedConfig = JSON.parse(atob(storedConfiguration));

if (!parsedConfig[0].minW) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we validate here that parsedConfig.length > 0 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we define that value internally to position the widgets, it will always contain a first element unless we delete it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added an extra validation

@bexsoft
Copy link
Collaborator Author

bexsoft commented Jan 22, 2021

LGTM, it just have some compilation warnings

Compiled with warnings.

./src/screens/Console/Dashboard/Prometheus/PrDashboard.tsx
  Line 17:28:  'useMemo' is defined but never used                                                                                  @typescript-eslint/no-unused-vars
  Line 159:5:  React Hook useCallback has a missing dependency: 'dashboardDistr'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

./src/screens/Console/Dashboard/Prometheus/Widgets/LinearGraphWidget.tsx
  Line 81:18:  Expected '!==' and instead saw '!='  eqeqeq

fixed

@bexsoft bexsoft requested a review from Alevsk January 22, 2021 05:11
@bexsoft bexsoft added the WIP This PR is WIP and cannot be merged yet label Jan 22, 2021
@bexsoft bexsoft force-pushed the style-improvements-dashboard branch from a0d4c76 to d091119 Compare January 22, 2021 20:04
Alevsk
Alevsk previously approved these changes Jan 22, 2021
Alevsk
Alevsk previously approved these changes Jan 23, 2021
-Added padding to the bottom of dashboard

-Added calculations for linear chart tick interval

-Added default min width configurations to panels.

- Fixed crash on clean tenant
@bexsoft bexsoft force-pushed the style-improvements-dashboard branch from 60eed86 to 94c18dd Compare January 23, 2021 02:20
@bexsoft bexsoft removed the WIP This PR is WIP and cannot be merged yet label Jan 23, 2021
@bexsoft bexsoft requested a review from Alevsk January 23, 2021 02:31
Copy link
Contributor

@Alevsk Alevsk left a comment

Choose a reason for hiding this comment

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

All issues addressed, LGTM

@dvaldivia dvaldivia merged commit 52fac7f into minio:master Jan 25, 2021
@bexsoft bexsoft deleted the style-improvements-dashboard branch January 25, 2021 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants