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

[ENH] Implement Notification Icon with Badge for Warnings and Its Test #432

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Tusharjamdade
Copy link

@Tusharjamdade Tusharjamdade commented Jan 8, 2025

Changes proposed in this pull request:

  • Implemented a notification badge for non-critical warnings
  • Additionally, added a test for the notification badge

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass
  • If the PR changes the participant-level and/or the dataset-level result file, the query-tool-results files in the neurobagel_examples repo have been regenerated

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

Summary by Sourcery

Add a notification icon with a badge to display non-critical warnings to the user.

New Features:

  • Display non-critical warnings as notifications in a popover.

Tests:

  • Add tests for the notification icon and badge.

Copy link

sourcery-ai bot commented Jan 8, 2025

Reviewer's Guide by Sourcery

This pull request implements a notification icon with a badge in the navigation bar to display non-critical warnings to the user. It also includes a test for the new notification badge feature.

Sequence diagram for the new warning notification flow

sequenceDiagram
    participant User
    participant App
    participant Navbar
    participant NotificationSystem

    App->>NotificationSystem: Initialize warnings state
    App->>Navbar: Render with notifications prop
    Note over Navbar: Display notification badge
    User->>Navbar: Click notification icon
    Navbar->>NotificationSystem: Open notification popover
    NotificationSystem-->>User: Display warning messages
    User->>NotificationSystem: View warnings
    User->>NotificationSystem: Close notification popover
Loading

Class diagram for updated Navbar component

classDiagram
    class Navbar {
        +boolean isLoggedIn
        +function onLogin()
        +string[] notifications
        -HTMLElement anchorEl
        -HTMLElement anchorMailEl
        -boolean openAccountMenu
        -boolean openMailMenu
        +handleClick(event)
        +handleClose()
        +handleMailClick(event)
        +handleMailClose()
    }
    note for Navbar "Added notifications support"

    class Badge {
        +number badgeContent
        +string color
    }

    class NotificationsIcon {
        +string color
    }

    Navbar --> Badge
    Navbar --> NotificationsIcon
Loading

File-Level Changes

Change Details Files
Added a notification icon and badge to the navigation bar
  • Introduced a notification icon with a badge to display the number of warnings.
  • The notification icon opens a popover that lists the warning messages.
  • Included warning messages in the state and updated them when errors occur during data fetching.
  • Added tests to check the visibility of the notification icon and badge.
src/components/Navbar.tsx
src/App.tsx
cypress/component/Navbar.cy.tsx
Updated the Navbar component to handle notifications
  • Added a new notifications prop to the Navbar component to receive warning messages.
  • Implemented the popover functionality to display the list of warnings when the notification icon is clicked.
src/components/Navbar.tsx
Implemented warning handling in the App component
  • Added a warnings state variable to store warning messages.
  • Updated the error handling logic to add warning messages to the state instead of displaying snackbars.
  • Passed the warnings state to the Navbar component as a prop.
src/App.tsx
Added tests for the notification badge
  • Added tests to verify that the notification icon and badge are visible in the navigation bar.
cypress/component/Navbar.cy.tsx

Assessment against linked issues

Issue Objective Addressed Explanation
#393 Provide a way to remove or hide non-critical warnings from the UI
#393 Make current warning behavior a 'debug' mode and default to 'off' The PR implements a notification badge for warnings, but does not explicitly create a 'debug' mode or change the default warning display behavior
#393 Show only a little counter of warnings and errors with a badge that can be expanded on click

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions github-actions bot added the _community [BOT ONLY] PR label for community contributions. Used for tracking label Jan 8, 2025
Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for neurobagel-query ready!

Name Link
🔨 Latest commit 470b988
🔍 Latest deploy log https://app.netlify.com/sites/neurobagel-query/deploys/677ebcabd05c310008b685aa
😎 Deploy Preview https://deploy-preview-432--neurobagel-query.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
_community [BOT ONLY] PR label for community contributions. Used for tracking
Projects
Status: Community
Development

Successfully merging this pull request may close these issues.

Make non-critical warnings less intrusive
1 participant