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

Support select till the end of the file / till the start of the file keyboard actions on Windows #989

Merged
merged 3 commits into from Jan 12, 2024

Conversation

Schahen
Copy link
Collaborator

@Schahen Schahen commented Jan 11, 2024

This PR is addressing following task
https://youtrack.jetbrains.com/issue/COMPOSE-784/Enable-selecting-till-the-end-of-the-file-till-the-beginning-of-the-file-in-Windows

(As of now, we do not support the standard way of selecting from current position till the end of the text / till the beginning of the text (that is - Shift / Ctrl / End and Shift / Ctrl / Home respectively) - but in MacOS we actually do. This goal of this task is to introduce such support)

Main changes are following:

  • In KeyMapping.skikoMain.kt additional mapping is introduced and used instead defaultKeyMapping on. desktop (which can be done later on, it's just that we usually - this is at least my understanding - avoid intrusive changes to commonMain as the very first step if possible)
  • SelectionTest rewritten so there'll be less clutter whenever we are adding new one

@Schahen Schahen requested review from igordmn and eymar January 11, 2024 15:29
else -> defaultKeyMapping
else -> defaultSkikoKeyMapping
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we just modify defaultKeyMapping in common?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@MatkovIvan I'd ve glad to I just thought that this is less recommended because we need to merge back and to coordinate any changes - no matter how small - with google team.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If the code is platform specific, better to keep it in our source set only.

If the code isn't platform specific - better to change it in commonMain and upstream.

Key mapping usually is platform specific, even if we have some default. We can't tell if it is needed for Android. defaultKeyMapping was added only because we didn't want to investigate platform behaviours.

else -> defaultKeyMapping
else -> defaultSkikoKeyMapping
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the code is platform specific, better to keep it in our source set only.

If the code isn't platform specific - better to change it in commonMain and upstream.

Key mapping usually is platform specific, even if we have some default. We can't tell if it is needed for Android. defaultKeyMapping was added only because we didn't want to investigate platform behaviours.

@Schahen Schahen changed the title Compose 784 select home end win pr [COMPOSE-784] Support select till the end of the file / till the start of the file keyboard actions on Windows Jan 12, 2024
@Schahen Schahen changed the title [COMPOSE-784] Support select till the end of the file / till the start of the file keyboard actions on Windows COMPOSE-784 Support select till the end of the file / till the start of the file keyboard actions on Windows Jan 12, 2024
@igordmn igordmn changed the title COMPOSE-784 Support select till the end of the file / till the start of the file keyboard actions on Windows Support select till the end of the file / till the start of the file keyboard actions on Windows Jan 12, 2024
@Schahen Schahen merged commit 8092be4 into jb-main Jan 12, 2024
6 checks passed
@Schahen Schahen deleted the COMPOSE-784-select-home-end-win-pr branch January 12, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants