Skip to content
This repository was archived by the owner on Dec 16, 2024. It is now read-only.

Conversation

@gilbertorconde
Copy link

@gilbertorconde gilbertorconde commented Apr 11, 2023

Add extended pt layout

Copy link
Owner

@nick-shmyrev nick-shmyrev left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, overal it looks great! Just a couple of tiny issues:

Lowercase level:
image

Uppercase level:
image

The "hide" key seems to "jump around" when you switch between lowercase and uppercase layers, and the arrow keys seem to be misaligned in uppercase level, see the 2 screenshots above.

Symbols layer:
image

I see you've tried to make sure the Symbols layer keys line up properly, nice! There's a small gap left of the "ABC" key, adding 0.5 to space bar width should hopefully fix that 👍

@gilbertorconde
Copy link
Author

gilbertorconde commented Apr 18, 2023

Greate :D did you look at my last commit? Because I noticed those issues right after my PR and fix those. Now I'm not in my computer but I'll check again asap

@nick-shmyrev
Copy link
Owner

Yep, I checked out the last commit on your PR. Perhaps that fix wasn't pushed upstream?

@nick-shmyrev nick-shmyrev self-requested a review April 19, 2023 15:54
@nick-shmyrev
Copy link
Owner

Also, something that worries me is that now the "Hide" button placement is going to be inconsistent, i.e. it'll be in one place on the first 2 layers, and in a different place on the rest of the PT layers as well as all other language layouts. I understand why you wanted to move it to a different spot, but I'm a bit worried that having to remember where it is on different layers might get annoying.
This is not necessarily a change request, but rather something to think about. In the end, you (and other people using PT layout) are going to have to live with it.

@gilbertorconde
Copy link
Author

Also, something that worries me is that now the "Hide" button placement is going to be inconsistent, i.e. it'll be in one place on the first 2 layers, and in a different place on the rest of the PT layers as well as all other language layouts. I understand why you wanted to move it to a different spot, but I'm a bit worried that having to remember where it is on different layers might get annoying.
This is not necessarily a change request, but rather something to think about. In the end, you (and other people using PT layout) are going to have to live with it.

You're right. I'll think about that as well

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants