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

Inconsistencies in new Container figure #441

Open
ptziegler opened this issue May 13, 2024 · 6 comments
Open

Inconsistencies in new Container figure #441

ptziegler opened this issue May 13, 2024 · 6 comments

Comments

@ptziegler
Copy link
Contributor

This is an umbrella issue to keep track of the odd behavior that can be observed when working with the Container figure.

  • Because borders are not drawn, the call to setBorder() should be ignored. Otherwise the insets of the border are still used when performing the layout, leading to "ghost" edges.

  • Because the background is not drawn, the call to setOpaque() should be ignored.

Not necessarily wrong, but still weird:

  • When the layout is set via the constructor, it seems weird to change it later on. Should the call to setLayoutManager() also be ignored?
ptziegler referenced this issue May 13, 2024
Using pure Figure just as layout container for other figures is very
heavy weight as the graphics state is saved and restored very often. For
pure layout containers this would not be necessary. This new figure can
help to reduce memory footprint and drawing performance for these cases.
@azoitl
Copy link
Contributor

azoitl commented May 13, 2024

I found the following additional topics to be considered for ignoring:

  1. Tooltip
  2. Cursor
  3. Getting or requesting fokus
  4. Any Mouse, key, or fokus event

@azoitl
Copy link
Contributor

azoitl commented May 13, 2024

Not necessarily wrong, but still weird:

  • When the layout is set via the constructor, it seems weird to change it later on. Should the call to setLayoutManager() also be ignored?

I mainly added the layout to the constructor to save a line when using Container. In order to reuse existing setLayoutManager() infrastructure I made the setLayoutManager() final.

I think ignoring a call to setLayoutManager() would feel awkward to me.

@ptziegler
Copy link
Contributor Author

I mainly added the layout to the constructor to save a line when using Container. In order to reuse existing setLayoutManager() infrastructure I made the setLayoutManager() final.

I think ignoring a call to setLayoutManager() would feel awkward to me.

I don't mind either way. But after looking at it again, I see the layout manager being passed as a constructor argument, which isn't done for any other figure. So I automatically assign it some special meaning, which makes it seem weird that it can easily be replaced.

@azoitl
Copy link
Contributor

azoitl commented May 16, 2024

I mainly added the layout to the constructor to save a line when using Container. In order to reuse existing setLayoutManager() infrastructure I made the setLayoutManager() final.
I think ignoring a call to setLayoutManager() would feel awkward to me.

I don't mind either way. But after looking at it again, I see the layout manager being passed as a constructor argument, which isn't done for any other figure. So I automatically assign it some special meaning, which makes it seem weird that it can easily be replaced.

My thoughts where that a Container can only be used to layout children so using it without LayoutManager makes no sense. This is different to other figures. On the other hand one may want to change the LayoutManager based on some user action (e.g. flipping from row to column layout). Removing the container and adding a new one seems odd for this.

Copy link

This issue is stale because it has been open for 90 days with no activity.

@github-actions github-actions bot added the stale label Aug 15, 2024
@ptziegler ptziegler removed the stale label Aug 22, 2024
Copy link

This issue is stale because it has been open for 90 days with no activity.

@github-actions github-actions bot added the stale label Nov 21, 2024
@ptziegler ptziegler removed the stale label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants