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

[VisUI] Root group parameter for ToastManager #279

Closed
metaphore opened this issue Nov 28, 2017 · 8 comments
Closed

[VisUI] Root group parameter for ToastManager #279

metaphore opened this issue Nov 28, 2017 · 8 comments
Labels

Comments

@metaphore
Copy link
Contributor

metaphore commented Nov 28, 2017

ToastManager is parametrized with Stage and adds new toasts into stage's root group. But unfortunately this leads to the overlapping problem when there is another kind of generic widgets that are added to the stage just as into the hip. I usually experience it when new dialog appears on the stage (e.g. LML Autumn creates them on the stage's root).

From the ToastManager code I see this should not be a problem to use Group field instead of Stage to host toasts into it. Like we can keep old constructor, but just extract root group from the stage for an old code compatibility:

private final Group root;

public ToastManager (Group root) {
    this.root = root;
}

public ToastManager (Stage stage) {
    this.root = stage.getRoot();
}

I can prepare PR if you would like, but I just feel like it's better to discuss first.

@kotcrab
Copy link
Owner

kotcrab commented Nov 28, 2017

ToastManager is using Stage to get width and height of it using: stage.getWidth() / getHeight() so alignment to edges can be done. stage.getRoot().getWidth() returns 0 because Group does not implement calculating it if I remember correctly.

Not sure how to handle this.

  • Passing width and height to resize would break API.
  • Gdx.graphics.getWidth() is not necesary the same as stage width.

@kotcrab kotcrab added the ui label Nov 28, 2017
@metaphore
Copy link
Contributor Author

metaphore commented Nov 28, 2017

Oh, you're right stage's root always has zero size, my mistake.
Hm, the shortest solution I can think of:

public ToastManager (Stage stage) {
    WidgetGroup toastHost = new WidgetGroup();
    toastHost.setFillParent(true);
    stage.addActor(toastHost);
    this.root = toastHost;
}

This should work for both compatibility and resize sakes.

@kotcrab
Copy link
Owner

kotcrab commented Nov 28, 2017

Yep, I think this will work. I also thought about using root.getStage() but yours allows to do alignment within group bounds properly. However the alignment code will need to be changed, it now must use Group X/Y position.

PR would be greatly appreciated. Please also update ui\CHANGES.md and feel free to add yourself to CONTRIBUTORS.md

@metaphore
Copy link
Contributor Author

@kotcrab good point, will do!

@metaphore
Copy link
Contributor Author

It turned out that we don't need to pay attention to root group's X/Y because toast positions are relative to a parent and it's totally fine when root is located not in stage's zero or just moving.

@kotcrab
Copy link
Owner

kotcrab commented Nov 29, 2017

That's great, merged PR.

@jojorabbit
Copy link

Hi,
just a small comment.
This does not seem the perfect solution.
In case of adding toastManager after main root/actor to a stage, setFillParent will cover main root and events will not work on the main root since it is covered with widgetGroup from the ToastManager.
The workaround is to add toastManager root to stage e.g. create toastManager first and add main root/actor to a stage but this would probably cause other problems e.g. toast root covered by main.

You can add
widgetGroup.setTouchable(Touchable.childrenOnly);
so events don't trigger on the toast widget group just on children.

Let me know what you think.
Cheers.

@metaphore
Copy link
Contributor Author

@jojorabbit oh you're right, I missed that one. Thanks!

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

No branches or pull requests

3 participants