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

Bug/Remove bubble when no reviews #807

Merged
merged 3 commits into from
Oct 26, 2023
Merged

Bug/Remove bubble when no reviews #807

merged 3 commits into from
Oct 26, 2023

Conversation

GB27247
Copy link
Member

@GB27247 GB27247 commented Oct 24, 2023

No description provided.

@@ -147,12 +147,12 @@ export default function SideNavigation({
<ListItemIcon>
{!drawerOpen ? (
<Tooltip title='Review' arrow placement='right'>
<Badge badgeContent={reviewCount} color='secondary'>
<Badge badgeContent={reviewCount} color='secondary' invisible={reviewCount == 0}>
Copy link
Member

Choose a reason for hiding this comment

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

Always use strict equality === when comparing values.

@@ -91,7 +91,7 @@ export default function SideNavigation({
}, [])

async function fetchReviewCount() {
setReviewCount((await getReviewCount()).headers.get('x-count') as unknown as number)
setReviewCount((await getReviewCount()).headers.get('x-count') as string)
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be cleaner to parse it as a number here. E.g:

setReviewCount(parseInt((await getReviewCount()).headers.get('x-count')))

Then reviewCount can remain a number, and you can compare it as a number

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it be preferable to raise an error on a null response or set the value to 0?

Copy link
Member

@ARADDCC002 ARADDCC002 Oct 25, 2023

Choose a reason for hiding this comment

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

IMO we should probably send some sort of feedback if it can't find it and then set it to 0 so that it hides the bubble.

We use the useNotification hook as defined here to show snackbar type feedback to user. There's an example of implementation here

@GB27247 GB27247 merged commit 018807a into main Oct 26, 2023
@ARADDCC012 ARADDCC012 deleted the bug/remove-review-bubble branch October 30, 2023 10:36
@ARADDCC012 ARADDCC012 restored the bug/remove-review-bubble branch October 30, 2023 10:36
@ARADDCC012 ARADDCC012 deleted the bug/remove-review-bubble branch October 31, 2023 15:14
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.

3 participants