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

feat: add timeout method for ui.notification #4444

Merged
merged 5 commits into from
Mar 12, 2025

Conversation

weinibuliu
Copy link
Contributor

@weinibuliu weinibuliu commented Mar 8, 2025

This PR is based on #4437 , which explores a way to make notifications dismiss after a certain period of time.

This PR provides a method called timeout that allows people to directly override the value of timeout (instead of manually modifying the value of the props)

I did a simple test.It worked well.

# notification.py (This PR did)
class Notification(Element, component='notification.js'):
    ...
    @property
    def timeout(self) -> Optional[float]:
        """The timout value of the notification."""
        return self._props['options']['timeout']

    @timeout.setter
    def timeout(self,value: Optional[float]) -> None:
        self._props["options"]["timeout"] = (value or 0) *1000
        self.update()

# test.py
import asyncio

from nicegui import ui

async def test_set_timeout():
    n = ui.notification("Test Notification",timeout=None)
    n.on_dismiss(lambda:print("Notification Dismiss."))
    await asyncio.sleep(3)
    n.message="Test Done"
    n.timeout = 3 # This is a new method

ui.button("Test").on_click(test_set_timeout)

ui.run()

That's all.

If you have any suggestions for changes to this PR, please let me know. Thanks a lot.

@weinibuliu weinibuliu marked this pull request as draft March 8, 2025 06:22
@falkoschindler falkoschindler self-requested a review March 8, 2025 08:28
@weinibuliu weinibuliu marked this pull request as ready for review March 8, 2025 08:37
@weinibuliu
Copy link
Contributor Author

weinibuliu commented Mar 8, 2025

Hi, @falkoschindler
I'm thinking of a method called reset that will allow developers to change almost any value of a notification by calling a single line of code. It will work like below.
Please note that this is just a demo and it is not included in this PR.

# notification.py
class ChangeType(Enum):
    NoneChange = auto() # Make sure people don't provide values ​​that are equal to it

class Notification(Element, component='notification.js'):
    ...
    def reset(self,
              message: Any = ChangeType.NoneChange,
              position: Union[NotificationPosition, ChangeType] = ChangeType.NoneChange,
              type: Union[NotificationType, ChangeType] = ChangeType.NoneChange,
              timeout: Union[float, int, ChangeType] = ChangeType.NoneChange,) -> None: # For a demo,I only use some params.
        
        self.props["options"]["message"] = str(message) if message != ChangeType.NoneChange  else self.props["options"]["message"]
        self.props["options"]["position"] = position if position != ChangeType.NoneChange else self.props["options"]["position"]
        self.props["options"]["type"] = type if type != ChangeType.NoneChange else self.props["options"]["type"]
        self.props["options"]["timeout"] = (timeout or 0)*1000 if timeout != ChangeType.NoneChange else self.props["options"]["timeout"]
        self.update()

Developers only need to call it like this.

# test.py
import asyncio

from nicegui import ui

async def test_reset():
    n = ui.notification("Test Notification",timeout=None,position="bottom-right",type="info")
    n.on_dismiss(lambda:print("Notification Dismiss."))
    await asyncio.sleep(3) # Do something
    n.reset("Test Done",timeout=5,type="positive") # This is a new method

ui.button("Test").on_click(test_reset)

ui.run()
n.reset("Test Done",timeout=5,type="positive")
# It is approximately equivalent to the following code.
n.message = "Test Done"
n.timeout = 5
n.type = "positive"

I think this will be more convenient, can we expand on this? If you have any suggestions for improvements to this idea, please let me know, thank you very much.

@weinibuliu weinibuliu changed the title feat: add set_timeout for ui.notification feat: add timeout method for ui.notification Mar 8, 2025
@falkoschindler falkoschindler added the enhancement New feature or request label Mar 12, 2025
@falkoschindler falkoschindler added this to the 2.13 milestone Mar 12, 2025
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request, @weinibuliu!
I cleaned it up a bit and now it's ready to merge.

Regarding a reset method: I'm a bit hesitant because we'll basically need to duplicate the signature and docstring of the initializer. But I guess it would be a useful feature nonetheless. Would you like to create a separate pull request?

@falkoschindler falkoschindler merged commit 8a44759 into zauberzeug:main Mar 12, 2025
1 check passed
@weinibuliu weinibuliu deleted the feat/notification branch March 12, 2025 09:17
@weinibuliu
Copy link
Contributor Author

Thanks for the pull request, @weinibuliu! I cleaned it up a bit and now it's ready to merge.

Regarding a reset method: I'm a bit hesitant because we'll basically need to duplicate the signature and docstring of the initializer. But I guess it would be a useful feature nonetheless. Would you like to create a separate pull request?

Thank you very much for your cleanup and approval.
About reset, should we add it only for the notification component, or for all components? I think it won't be too difficult to add it to all components, but is it necessary for us to do so? If the community feels it is necessary after discussion, I am happy to create a PR.

@weinibuliu
Copy link
Contributor Author

I have created a new discussion on it. #4460

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants