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

Ability to programmatically close a specific notification #1801

Closed
1 of 6 tasks
zmikea opened this issue Sep 18, 2024 · 17 comments · Fixed by #1985
Closed
1 of 6 tasks

Ability to programmatically close a specific notification #1801

zmikea opened this issue Sep 18, 2024 · 17 comments · Fixed by #1985
Assignees
Labels
🖰 GUI Related to GUI hacktoberfest - 100💎 Issues rewarded by 100 points hacktoberfest hacktoberfest issues 🆘 Help wanted Open to participation from the community 📈 Improvement Improvement of a feature. 🟨 Priority: Medium Not blocking but should be addressed ⚔️ Quest Tracks quest-bot quests 📝Release Notes Impacts the Release Notes or the Documentation in general

Comments

@zmikea
Copy link

zmikea commented Sep 18, 2024

Description

I have potentially 3 notifications during a process, and at the end of the process want to programmatically close a specific one of them immediately, but leave the other two to close based on their duration.

Solution Proposed

create an ID for each notification upon creation so that a user can obtain it upon creation, example:
id = notify(state, 'info', 'blah blah', duration=10000)
And then later in code, they can close it with the line
id.close

Impact of Solution

No response

Additional Context

No response

Acceptance Criteria

  • Ensure new code is unit tested, and check code coverage is at least 90%.
  • Create related issue in taipy-doc for documentation and Release Notes.
  • Check if a new demo could be provided based on this, or if legacy demos could be benefit from it.
  • Ensure any change is well documented.

Code of Conduct

  • I have checked the existing issues.
  • I am willing to work on this issue (optional)
@jrobinAV jrobinAV changed the title <write a small description here>ability to programmatically close a specific notification Ability to programmatically close a specific notification Sep 20, 2024
@jrobinAV jrobinAV added 📈 Improvement Improvement of a feature. 🖰 GUI Related to GUI 🟨 Priority: Medium Not blocking but should be addressed 📝Release Notes Impacts the Release Notes or the Documentation in general 🆘 Help wanted Open to participation from the community and removed ✨New feature labels Sep 20, 2024
@cu8code
Copy link

cu8code commented Sep 22, 2024

Hey @jrobinAV if this is up for grab, would like to work on this

@jrobinAV
Copy link
Member

Hello @cu8code,

Yes, it is. Let me assign it to you.

Thank you,

@cu8code
Copy link

cu8code commented Sep 23, 2024

@jrobinAV can you tell me which part of the code should I focus on. Just some of your initial thoughts about the problems. And the implementation!

@jrobinAV
Copy link
Member

HI,

I believe @FredLL-Avaiga @FabienLelaquais @dinhlongviolin1 @namnguyen20999 are the key people to help you.

@FredLL-Avaiga
Copy link
Member

right now there is no identifier for a notification
I don't know if the notistack package proposes this feature (deletion)
If it does I suppose we should create a new type of notification (so that we use the same ws message type) and implement it on the frontend depending on the way the package works

@FabienLelaquais
Copy link
Member

FabienLelaquais commented Sep 24, 2024

If a notification CAN be closed from the front-end... (which I think is pretty easy).
We could add an option "identifier" parameter to 'notify()' and send it along with the ALERT message, store it as a (data?) property of the generated element.
Then remove it when a new message (should we reuse ALERT?) is received referring to that id.
@FredLL-Avaiga, thoughts?

@FredLL-Avaiga
Copy link
Member

after reading the code:

  • there's already a way to remove the last notification by passing type= ""
  • notistack support a key entry in the option object.
    So we only need to add an id to the payload and set it if present
    Should be quite easy

@FabienLelaquais
Copy link
Member

... that sounds great. "Last notification" use case already handled (which I suspect is 99% of the cases).
Now we will want to add some semantics to 'duration': like "I 0, leave the notification on the page as long as it was not explicitly closed".

@cu8code
Copy link

cu8code commented Sep 25, 2024

@FabienLelaquais @jrobinAV reading the code

  • Every time we send a notification, we will send an additional ID
  • we need to return an object, that will have a close method
  • we could also just return the notification_ID, create a new function to close the notification taking notification_ID as parameter.
    def notify(

Also, how do I visually test, all the different changes, do I have to build each of the models, and then create an example, to visually check everything is working ?

@FredLL-Avaiga
Copy link
Member

no need to return an object
if a dev wants to be able to close a notification:

  • first notify with an id (ie add an optional id to notify)
  • when suitable, call the close_notification(state, id) (the frontend, will act accordingly).

@FredLL-Avaiga
Copy link
Member

for test, yes, you're supposed to test every (reasonable) possibility

@jrobinAV jrobinAV added hacktoberfest hacktoberfest issues hacktoberfest - 200💎💎 Issues rewarded by 200 points and removed hacktoberfest hacktoberfest issues hacktoberfest - 200💎💎 Issues rewarded by 200 points labels Sep 25, 2024
@FredLL-Avaiga FredLL-Avaiga added hacktoberfest hacktoberfest issues hacktoberfest - 100💎 Issues rewarded by 100 points labels Oct 6, 2024
@quest-bot quest-bot bot added the ⚔️ Quest Tracks quest-bot quests label Oct 7, 2024
Copy link

quest-bot bot commented Oct 7, 2024

New Quest! image New Quest!

A new Quest has been launched in @Avaiga’s repo.
Merge a PR that solves this issue to loot the Quest and earn your reward.


Some loot has been stashed in this issue to reward the solver!

🗡 Comment @quest-bot embark to check-in for this Quest and start solving the issue. Other solvers will be notified!

⚔️ When you submit a PR, comment @quest-bot loot #1801 to link your PR to this Quest.

Questions? Check out the docs.

@cu8code cu8code removed their assignment Oct 8, 2024
@AdeshGhadage
Copy link
Contributor

I want to work on this issue can you assign me ? @jrobinAV

@FlorianJacta
Copy link
Member

Here you go @AdeshGhadage !

@AdeshGhadage
Copy link
Contributor

@quest-bot embark

Copy link

quest-bot bot commented Oct 11, 2024

@AdeshGhadage has embarked on their Quest. 🗡

  • @AdeshGhadage has been on GitHub since 2022.
  • They have merged 11 public PRs in that time.
  • Their swords are blessed with CSS and HTML magic ✨
  • They have contributed to this repo before.

This is not an assignment to the issue. Please check the repo’s contribution guidelines before submitting a PR.

Questions? Check out the docs.

Copy link

quest-bot bot commented Oct 11, 2024

🧚 @AdeshGhadage has submitted PR #1985 and is claiming the loot.

Keep up the pace, or you'll be left in the shadows.

Questions? Check out the docs.

AdeshGhadage added a commit to AdeshGhadage/taipy that referenced this issue Oct 11, 2024
AdeshGhadage added a commit to AdeshGhadage/taipy that referenced this issue Oct 14, 2024
AdeshGhadage added a commit to AdeshGhadage/taipy that referenced this issue Oct 14, 2024
AdeshGhadage added a commit to AdeshGhadage/taipy that referenced this issue Oct 21, 2024
AdeshGhadage added a commit to AdeshGhadage/taipy that referenced this issue Oct 21, 2024
FredLL-Avaiga added a commit to AdeshGhadage/taipy that referenced this issue Oct 21, 2024
FredLL-Avaiga added a commit to AdeshGhadage/taipy that referenced this issue Oct 22, 2024
AdeshGhadage added a commit to AdeshGhadage/taipy that referenced this issue Oct 23, 2024
jrobinAV pushed a commit that referenced this issue Oct 23, 2024
* Added close a specific notification feature

* ruff space removal

* removed new __send_ws_alert

* comments removed

* add frontend and changed in notification id

* changed notification id by nanoid

* sapce correction

* resolved multiple issue

* return state._gui

* fixed useref error and added test for deleteAlert

* changed nanoid to random string

---------

Co-authored-by: Fred Lefévère-Laoide <[email protected]>
@github-actions github-actions bot added the 🥶Waiting for contributor Issues or PRs waiting for a long time label Oct 25, 2024
@FredLL-Avaiga FredLL-Avaiga removed the 🥶Waiting for contributor Issues or PRs waiting for a long time label Oct 25, 2024
@Avaiga Avaiga deleted a comment from github-actions bot Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖰 GUI Related to GUI hacktoberfest - 100💎 Issues rewarded by 100 points hacktoberfest hacktoberfest issues 🆘 Help wanted Open to participation from the community 📈 Improvement Improvement of a feature. 🟨 Priority: Medium Not blocking but should be addressed ⚔️ Quest Tracks quest-bot quests 📝Release Notes Impacts the Release Notes or the Documentation in general
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants