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

feat: ESC key selects parent of selected node #389

Merged
merged 5 commits into from
Sep 30, 2024

Conversation

luca-domenichini
Copy link
Contributor

This PR implements #388.
Pressing ESC key selects the parent of the currently selected node.

@abhinayagarwal abhinayagarwal added the enhancement New feature or request label Aug 31, 2021
@abhinayagarwal abhinayagarwal added this to the 18 milestone Sep 1, 2021
@abhinayagarwal abhinayagarwal changed the title Implementation for #388 ESC keypress should select parent of selected node Sep 23, 2021
@luca-domenichini
Copy link
Contributor Author

@abhinayagarwal any news on this?

@AlmasB AlmasB changed the title ESC keypress should select parent of selected node feat: ESC key selects parent of selected node Feb 18, 2022
Copy link
Collaborator

@AlmasB AlmasB left a comment

Choose a reason for hiding this comment

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

Looks good, thanks. Please see below

@@ -85,6 +88,22 @@ protected void initializePanel() {
// We do not use the platform editing feature because
// editing is started on selection + simple click instead of double click
treeView.setEditable(false);

treeView.setOnKeyPressed(event -> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any reason for why this is setOnKeyPressed() and the other change is addEventFilter()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, sorry, I did not receive any mail about your comment. Let me check in the next days.

Copy link
Contributor Author

@luca-domenichini luca-domenichini Mar 30, 2022

Choose a reason for hiding this comment

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

@AlmasB changed to always call setOnKerPressed()

Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler Oct 2, 2022

Choose a reason for hiding this comment

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

Technically this looks okay for me.

Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler left a comment

Choose a reason for hiding this comment

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

Please update first license header line in in ContentPanelController.java from * Copyright (c) 2016, 2018 Gluon and/or its affiliates. to * Copyright (c) 2016, 2022 Gluon and/or its affiliates..

Also please add the line * Copyright (c) 2022 Gluon and/or its affiliates. as new first line in HierarchyTreeViewController.java.

@@ -85,6 +88,22 @@ protected void initializePanel() {
// We do not use the platform editing feature because
// editing is started on selection + simple click instead of double click
treeView.setEditable(false);

treeView.setOnKeyPressed(event -> {
Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler Oct 2, 2022

Choose a reason for hiding this comment

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

Technically this looks okay for me.

@Oliver-Loeffler
Copy link
Collaborator

Hi @luca-domenichini ,
sorry that the review really took that long. Could you please update the license headers of both modified files?
Thanks for your contribution - I've played with it and it works fine.

@luca-domenichini
Copy link
Contributor Author

@Oliver-Loeffler done

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.

4 participants