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

πŸ’₯ Notifications got better. #325

Conversation

infinite-dev22
Copy link
Contributor

Notifications package.
πŸ”₯ Notifications no longer create a custom separate stage for them to show rather use the parent application's stage.
πŸ”₯ No seperate windows created for notification system
βœ… Top left corner nolonger blocked by notification stage window.

Notifications package.
πŸ”₯ Notifications no longer create a custom separate stage for them to show rather use the parent application's stage.
@stale stale bot added the stale label Jul 20, 2023
@stale stale bot closed this Jul 27, 2023
@palexdev palexdev reopened this Jul 28, 2023
Repository owner deleted a comment from stale bot Jul 28, 2023
@stale stale bot removed the stale label Jul 28, 2023
Repository owner deleted a comment from stale bot Jul 28, 2023
@palexdev palexdev added the enhancement New feature or request label Jul 28, 2023
@palexdev
Copy link
Owner

@infinite-dev22 Almost ready to merge, there's just a little mistake that needs to be fixed, I left a comment

@infinite-dev22
Copy link
Contributor Author

@palexdev Hope you are well, Sir. I didn't find the comment left on the mistake to be fixed. Thank you.

@palexdev
Copy link
Owner

@palexdev Hope you are well, Sir. I didn't find the comment left on the mistake to be fixed. Thank you.

Do you see it now?

@infinite-dev22
Copy link
Contributor Author

Yes, good Sir

@palexdev
Copy link
Owner

@infinite-dev22 can you make the change by the end of the week, so I continue working/release?

@infinite-dev22
Copy link
Contributor Author

Yes Sir, I can, Sorry for late response. Just a busy month for me. Let make the change

@infinite-dev22
Copy link
Contributor Author

Though when I fetch the change seems to already exist

@infinite-dev22
Copy link
Contributor Author

@palexdev, this has been resolved

@infinite-dev22
Copy link
Contributor Author

Though a few questions about the change made, does the height have to be manually set. Can this be set to scale with the notifications.

@palexdev
Copy link
Owner

Though a few questions about the change made, does the height have to be manually set. Can this be set to scale with the notifications.

Long story short: no, it has to be done manually

More details: Maybe a solution could be found with bindings but the container for the notifications is a VirtualFlow, so every notification must have the same height. Not only that, since it uses the old VirtualFlow implementation, the size is not a property of the flow but must be hard coded into the cell class

@infinite-dev22
Copy link
Contributor Author

infinite-dev22 commented Oct 28, 2023

More details: Maybe a solution could be found with bindings but the container for the notifications is a VirtualFlow, so every notification must have the same height.

I realized, I have tried setting the size automatically and the Notifications seemed to all be the same size as the first Notification displayed.
This then gets me to the next question, can the virtualflow be made(re written) to size according to its children dynamically with varying sizes just like HBox and VBox.

**Edit
I noticed this will be implemented in the future.

In the future I would also like to implement more complex Virtual Flows, one that doesn't care about cells sizes, one specifically for TreeViews and who knows what else.

@palexdev
Copy link
Owner

More details: Maybe a solution could be found with bindings but the container for the notifications is a VirtualFlow, so every notification must have the same height.

I realized, I have tried setting the size automatically and the Notifications seemed to all be the same size as the first Notification displayed.
This then gets me to the next question, can the virtualflow be made(re written) to size according to its children dynamically with varying sizes just like HBox and VBox.

**Edit
I noticed this will be implemented in the future.

In the future I would also like to implement more complex Virtual Flows, one that doesn't care about cells sizes, one specifically for TreeViews and who knows what else.

Yeah, easier said than done unfortunately.
There are several ways to implement it. It could rely on the 'infinite scroll' technique or it could use the 'cell size' property as the minimum size for each cell. The latter would allow us to use the same mechanism of the fixed one, with ranges and all, the layout and scroll would still be different.
There are also other ways that I can't remember now (it's 8AM here and I'm still sleepy) but none of them convince me at the moment either for performance reasons or feasibility reasons.
I guess there's only one way to find out, create a mock Node and test it interactively.
There's also another concern about the second solution I mentioned. Since we would use the 'cell size' as the minimum, which allows us to predict in the worst case scenario what the range of cells would be, it leaves me with the question "Would the user like this mechanism of minimum or is it too restrictive?"
It's also a bit of a performance hazard. What if the 'cell size' is set to be very small?

Long story short: VirtualizedFX things. are. complicated.

@palexdev palexdev merged commit 50126e4 into palexdev:main Oct 31, 2023
1 check passed
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