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

Switch back erroneously flipped layer palette mnemonics #4508

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ctrlcctrlv
Copy link
Member

Close #4504

Type of change

  • Bug fix

@skef
Copy link
Contributor

skef commented Nov 21, 2020

Original PR was #1103

@skef
Copy link
Contributor

skef commented Nov 21, 2020

I have to say that I don't much like either the "old" or the "new" picture here.

The new picture, which this PR reverses, is bad because the mnemonics are hard-coded and could conflict with assigned hotkeys.

The old picture is bad primarily because the text of the layer widget doesn't bother adding the underscores, meaning the user needs to know magic or dig deep into the documentation (?) to find them. It's also bad because it doesn't offer a hotkey interface.

In any case I'm confused about #4504 because it seems like the "new" code is stripping out ksm_meta, which should be Alt- (among other potential keys), so why do the two solutions conflict?

The ideal solution here would be to add a "CharView.Layer" section to the hotkey system where "CharView.Layer.Fore" identifies the foreground, "CharView.Layer.Blah" identifies the Blah layer, and so forth. Then we could add Shift-F and Shift-B to the default file and these strokes would be retained but not interfere with the Alt assignments. (With that option it seems fine to let the underscores slide.) However, that would be a bit of work.

I'm not going to approve because I think there should be some discussion of these issues before this gets merged.

@ctrlcctrlv
Copy link
Member Author

That's fine, no harm done. Often I open PRs to start discussion, as you know

@frank-trampe
Copy link
Contributor

I'm sort of inclined to bear those ills we have pending a complete fix.

@skef
Copy link
Contributor

skef commented Nov 28, 2020

@frank-trampe If we're bearing ills it's not clear to me that the later changes that this PR reverses don't win out. How is someone supposed to know about Alt-o?

@ctrlcctrlv ctrlcctrlv marked this pull request as draft December 13, 2020 07:30
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.

Keyboard shortcut of Select Fore Layer / Back Layer
3 participants