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

Make default dashboard #473

Conversation

LightOfHeaven1994
Copy link
Collaborator

@LightOfHeaven1994 LightOfHeaven1994 commented Nov 30, 2023

Changelog:

  1. Fixed warning by replacing Select property:
index.js:1 Warning: Invalid ARIA attribute `ariaLabelTypeAhead`. ARIA attributes follow the pattern aria-* and must be lowercase.
  1. Remove double negotiation !! for easier code read
  2. Make default dashboard work and show widgets. I found out that in Ibutsu we can set default dashboard for a project but it is not working. After PR merged, if you select project that has default dashboard - widgets will be shown. In case you select another dashboard than default and clear selection - default dashboard will be shown again.

Before:
Screencast from 2023-12-04 12-30-55

After:
Screencast from 2023-12-04 12-31-21

Copy link
Contributor

@rsnyman rsnyman left a comment

Choose a reason for hiding this comment

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

Looks good to me.

My only caveat is that in JS you use the !! to convert a value to a boolean. So while this probably still works, you may run into funny issues in the future. JS's truthiness can be a little quirky.

@LightOfHeaven1994
Copy link
Collaborator Author

Looks good to me.

My only caveat is that in JS you use the !! to convert a value to a boolean. So while this probably still works, you may run into funny issues in the future. JS's truthiness can be a little quirky.

aha, didn't know about it. Will return it back in next commit.
I need to fix other small thing as default widgets work now, I wish Select also has value set instead of being empty. I will put gif when finish with it "before" & "after".

Thanks for the review btw 👍

@LightOfHeaven1994 LightOfHeaven1994 force-pushed the make-default-dashboard-work branch from 98b1db6 to bf9dfaa Compare December 4, 2023 11:47
@LightOfHeaven1994 LightOfHeaven1994 force-pushed the make-default-dashboard-work branch from bf9dfaa to 829b3c2 Compare December 4, 2023 11:54
@LightOfHeaven1994 LightOfHeaven1994 marked this pull request as ready for review December 4, 2023 11:54
@LightOfHeaven1994
Copy link
Collaborator Author

Agree about converting to boolean, this sounds more safe. Also corrected placeholder of select in case there default dashboard exists

@LightOfHeaven1994 LightOfHeaven1994 merged commit 83b3393 into ibutsu:master Dec 4, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants