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

Add Close Button to Notifications and Improve Cosmetic Appearance #478

Closed
wants to merge 2 commits into from

Conversation

SO9010
Copy link
Contributor

@SO9010 SO9010 commented Jun 12, 2024

This merge request partially addresses issue #255, which I believe stems from an upstream issue. After exploring several solutions, including making the notification close when the user hovers over it, the problem persisted. I thoroughly reviewed all the code related to time handling to ensure that wasn't the problem.
Changes Made:

Added Close Button: Introduced a close button to the notification for more explicit user control.

Cosmetic Improvement: Made the notification slightly transparent for a better visual appeal.

These changes should enhance user experience by providing a clear way to dismiss notifications and a more polished look.

Here is a screenshot of it:

image

Please review and merge if everything looks good. Thank you!

@jacksongoode
Copy link
Collaborator

This is nice, thank you!

However, if this is to address a bug where notifications are not being cleared, I think it might be better to trigger some sort of notification clear method which gets rid of all the notifications. This could be as simple as pressing escape and triggering all the notifications to clear. I think it might be a little bit redundant to add the option to clear each individual notification given that they are intended to be ephemeral, temporary status messages. So I would advocate for a global clear rather than this individual UI change.

But even better would be to see why the the notifications are persisting in the first place.

@jacksongoode
Copy link
Collaborator

We could keep this or close it now that #503 is ready.

@SO9010
Copy link
Contributor Author

SO9010 commented Jul 24, 2024

I'll close it as it was only a temporary solution, as I thought it was an upstream issue.

@SO9010 SO9010 closed this Jul 24, 2024
@SO9010 SO9010 deleted the feature/rework-ui branch July 24, 2024 21:56
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