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

Implement Spacer And Add keyboard support to StripView #80

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

leewyatt
Copy link
Contributor

@leewyatt leewyatt commented Aug 21, 2023

@leewyatt leewyatt changed the title Implement Spacer Implement Spacer And Add keyboard support to StripView Aug 21, 2023
@@ -232,6 +232,24 @@ public void scrollTo(T item) {
getProperties().put("scroll.to", item);
}

/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

The java docs have to be added to the property() method, in this case loopSelectionProperty().

// If it's the last item and loop selection is off, don't consume the event
// so that focus can move to the next focusable component.
// Otherwise, select the next item.
if (index != itemCount - 1 || getSkinnable().isLoopSelection()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer the expression index < itemCount

case RIGHT:
case ENTER:
// Check if loop selection is enabled or if we haven't reached the last item yet
if (getSkinnable().isLoopSelection() || index < itemCount - 1) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace all getSkinnable() calls in this method with a previously assigned variable please.

@dlemmermann dlemmermann merged commit a324bf4 into dlsc-software-consulting-gmbh:master Aug 21, 2023
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.

2 participants