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

The close button of the notification does not work #284

Open
roy98 opened this issue Apr 20, 2022 · 7 comments · May be fixed by #314
Open

The close button of the notification does not work #284

roy98 opened this issue Apr 20, 2022 · 7 comments · May be fixed by #314
Assignees
Labels
bug Something isn't working

Comments

@roy98
Copy link

roy98 commented Apr 20, 2022

The close button of the notification does not work

This happens when you hover over the Notification box and click on the close button.
The Notification will no longer disappear.

Screen Shot 2022-04-20 at 23 53 46

@jennydaman jennydaman added bug Something isn't working ChRIS Store labels May 1, 2022
@jennydaman
Copy link
Collaborator

Taking a look at it now, there are several code smells in here.

https://github.com/FNNDSC/ChRIS_store_ui/blob/e07441dd8a940a76e558bd1a256c7ec691fdd184/src/components/Notification/index.jsx

closeable doesn't seem to be a useful prop, and it seems like there's nothing at the level of this component which implements hiding of the component.

@BhavyaT-135
Copy link

Hi @jennydaman , I'm new to this project and I would like to contribute. Can I take up this issue if no one else is working on it? 😊

@jennydaman
Copy link
Collaborator

@BhavyaT-135 sure. Reach out on Matrix if you'd like to chat about it.

@BhavyaT-135
Copy link

BhavyaT-135 commented Jun 26, 2022

Hi @jennydaman, yes I see that closeable has no use as of now and it is not affecting the hiding of the Alert Component. Should I declare it as a boolean state object and toggle it to false when the user clicks on the close button? 😊

@VictoriaAjala
Copy link

Hello @jennydaman , I'd like to work on this issue if it's still open.

@jennydaman
Copy link
Collaborator

jennydaman commented Oct 7, 2022

Sure @VictoriaAjala.

It's up to you whether you would like to attempt a quick fix or an overhaul of how these notification popups are implemented. The code could be redesigned to have a better interface/usage.

As always, reach out to me over Matrix @jennydaman:fedora.im if you have any questions.

@VictoriaAjala
Copy link

VictoriaAjala commented Oct 10, 2022

Thanks @jennydaman, I will get in touch with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants