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

some base level additions and minor cleanup and window title bar drag ability #498

Merged
merged 5 commits into from
Jul 25, 2020

Conversation

ginkoblongata
Copy link
Contributor

No description provided.

@ginkoblongata
Copy link
Contributor Author

This PR represents commonality sliced out of the PR of the ScrollPanel stuff.

Once this makes it through, I'll put in a PR for the ImageComponent, then another PR for the SplitPanel.

I think then the ScrolPanel PR will be hopefully on topic specifically enough.

Thanks!

@ginkoblongata ginkoblongata changed the base branch from master to release/3.1 June 28, 2020 18:25
@@ -44,6 +45,8 @@
private boolean strictFocusChange;
private boolean enableDirectionBasedMovements;
private Theme theme;

private Interactable mouseDownForDrag = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes it so that the component which experienced the mouse down is the component which the mouse drags get routed to.

if(globalPosition == null || lastKnownPosition == null) {
return null;
}
return globalPosition.withRelative(
-lastKnownPosition.getColumn() - contentOffset.getColumn(),
-lastKnownPosition.getRow() - contentOffset.getRow());
}

@Override
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used by MultiWindowTextGUI.

@@ -558,15 +557,15 @@ private Result handleEditableCBKeyStroke(KeyStroke keyStroke) {
public PopupWindow() {
setHints(Arrays.asList(
Hint.NO_FOCUS,
Hint.FIXED_POSITION));
Hint.FIXED_POSITION,
Hint.MENU_POPUP));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used by MultiWindowTextGUI.

@@ -46,12 +46,17 @@
private final WindowManager windowManager;
private final BasePane backgroundPane;
private final List<Window> windows;
private final List<Window> stableOrderingOfWindows;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose of the list stableOrderingOfWindows vs the windows list is that the WindowManager would prefer to have the same ordering everytime.

For example a 2x2 grid style window manager would like to arrange the windows in position and size and have those remain regardless of which window is active.
Prior to this change, the windows are coming through in different order because the active window is put to the front of the list, this would force most window managers to do this bookkeeping work which seems to involve overriding multiple methods.

So essentially z-order is what the existing windows list does, but most window managers would probably want access to z-ordering only secondarily to their primary ordering for layout purposes.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, but I think in that case rather than having two lists, we should create a WindowList class or something like that, which can encapsulate the z-index and the stable index. It would make the code interacting with the list(s) of windows in here much easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a WindowList class.

@ginkoblongata ginkoblongata changed the title doing some base level additions and minor cleanup doing some base level additions and minor cleanup, and window title bar drag ability Jun 28, 2020
@ginkoblongata ginkoblongata changed the title doing some base level additions and minor cleanup, and window title bar drag ability some base level additions and minor cleanup and window title bar drag ability Jun 28, 2020
@@ -180,6 +181,13 @@ public String toString() {
@Override
void invalidate();


default TerminalRectangle getBounds() {
Copy link
Owner

Choose a reason for hiding this comment

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

Need to add Javadoc to this one and the two below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated with some javadocs.

@@ -390,7 +398,11 @@ public String toString() {
* @return The global coordinates expressed as local coordinates
*/
@Override
@Deprecated
Copy link
Owner

@mabe02 mabe02 Jul 11, 2020

Choose a reason for hiding this comment

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

Add a @deprecated Javadoc note for this and explain what to use instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated with some javadocs.

@mabe02
Copy link
Owner

mabe02 commented Jul 11, 2020

Looks like we can merge this with a few tweaks. For some of the bigger changes in the other PR, I think we want to target master (3.2). But let's see, one PR at a time.

@ginkoblongata
Copy link
Contributor Author

ginkoblongata commented Jul 11, 2020

Ok, I think this is ready for next round. Once this is through, I think I can slice out the ImageComponent with pretty much cleanly for another PR.

@avl42
Copy link
Contributor

avl42 commented Jul 12, 2020 via email

@jrb0001
Copy link
Contributor

jrb0001 commented Jul 12, 2020

Java 8 introduced the default keyword on interface methods.

@ginkoblongata
Copy link
Contributor Author

Yes, also, by adding to the interface like that with the 'default' it makes it so that existing implementations of the interface can continue to work as is.
I tend to use that feature sparingly but it works out well for that.

I am hoping this PR will make it in.
I have a bunch of thoughts of potential rearrangement type breaking changes for Lanterna but I'd like to keep with the non breaking addition type stuff for now before focusing on that discussion. I think I've been leaning towards brevity for these but could go into more full context details.

@mabe02 mabe02 merged commit 2b7c23d into mabe02:release/3.1 Jul 25, 2020
@mabe02
Copy link
Owner

mabe02 commented Jul 25, 2020

Alright, that is now merged into 3.1

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

Successfully merging this pull request may close these issues.

4 participants