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

Fix #1397. Add show empty toggle to Dashboard #1398

Merged
merged 4 commits into from
Oct 4, 2021
Merged

Conversation

chenilim
Copy link
Contributor

@chenilim chenilim commented Oct 1, 2021

Summary

Add a "Show empty" toggle to the Dashboard to show / hide workspaces with no boards. This setting is persisted in localStorage.

Ticket Link

#1397

@chenilim chenilim requested a review from a team as a code owner October 1, 2021 18:49
@chenilim chenilim requested review from wiggin77 and sbishel and removed request for a team October 1, 2021 18:49
@wiggin77
Copy link
Contributor

wiggin77 commented Oct 1, 2021

@chenilim I think you need to update the test snapshot.

Copy link
Contributor

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

Snapshot still failing.

@chenilim
Copy link
Contributor Author

chenilim commented Oct 1, 2021

Yup, thanks. Looking into it. My environment seems to be borked, as jest is failing on something unrelated (props.contents.flat(...) is not a function). I'm going to try reinstalling, and may need to work on this later. Thanks.

@chenilim
Copy link
Contributor Author

chenilim commented Oct 1, 2021

Ok, turns out my node version was outdated. Updated the snapshot, and should be final now. Thanks.

@chenilim chenilim requested a review from wiggin77 October 1, 2021 21:55
Copy link
Contributor

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Collaborator

@sbishel sbishel left a comment

Choose a reason for hiding this comment

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

Just update 'userSettings.ts' for consistency. Otherwise LGTM.

static set dashboardShowEmpty(newValue: boolean) {
localStorage.setItem('dashboardShowEmpty', JSON.stringify(newValue))
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add the new item name to the UserSettingKey enum above? Also call the local methods for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done. I originally made the changes on the release-0.9.0 branch, and missed this enum.

@chenilim chenilim requested a review from sbishel October 2, 2021 01:12
@chenilim chenilim added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Oct 4, 2021
@chenilim chenilim merged commit 58f6b80 into main Oct 4, 2021
@chenilim chenilim deleted the gh-1397-show-empty branch October 4, 2021 21:27
@chenilim chenilim added the CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone label Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPick/Approved Meant for the quality or patch release tracked in the milestone CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants