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

UI and functionality remaining from the designs #23

Open
maria-komarova opened this issue Jan 13, 2021 · 34 comments
Open

UI and functionality remaining from the designs #23

maria-komarova opened this issue Jan 13, 2021 · 34 comments

Comments

@maria-komarova
Copy link

maria-komarova commented Jan 13, 2021

Trying to compare the configurator and the mockups that we had here are a few questions, notes on the differences I see. Some just ask for clarification.

  • The LED backlight color was shown with the border around the key in the designs. Unless we can make the backlight change on the actual keyboard as someone changes the setting in the configurator, we can try to implement showing it with the border. If there are no other technical limitations of course.
    For the state of the selected key - darker overlay on all the keys to make them uniform (I used the color of the text (#272727) at 35% opacity. But whatever gets us there visually works. Then the background of the key changes to white when it is selected and a checkmark appears in the bottom left corner. I believe we have our own styled checkmark filled with our orange. Only that shade of orange would change depending on the mode.
    If that is okay, let's try that first and if white looks too out of place for the dark mode, we'll figure something out.
    image

  • I still think the "instructions" in the left corner right above the keymap like on the mockup might be somewhat helpful. I know we planned that multi-selection with Shift + click is something mostly relevant for changing the backlight color if there is an ability to change it per key. So I guess that part depends on that. I think the first sentence should still be there though, mostly for the sake of users who might be seeing that kind of configurator for the first time. And please, let me know where we are on the per key backlight color for Launch. If we can't do that then maybe showing the backlight color with the border is not really that helpful either.
    image

  • We can move Load and Save to the hamburger menu in the top right corner. There were originally three options for saving in that menu - Save and Apply Changes, Save without Applying and Save as... "Save without Applying" should be disabled if the user is changing the current layout. One can either Save & apply Changes or Save as at this point. The reasoning was to make it easier to save, especially if we can use keyboard shortcut Ctrl + S to save the changes. I'm not sure where we are on that.
    There was also DIscard Changes (basically Reset), Rename Layout and Delete Layout there.
    image

  • That also brings up the question of could we use shortcuts we planned to use like Ctrl + S for saving and Ctrl + 1, Ctrl + 2, Ctrl + 3, and Ctrl + 4 for switching between 4 layers?

  • I can only see two layers at the moment. But I also see three more. I vaguely recall that at least one of them is only there for testing purposes, Electrical if I'm not mistaken. But I can't recall where we have left it with Keycaps and Logical.

  • The original placement of the dropdown that allows one to quickly switch between layouts was in the left top corner of the header (on the mockups). That still seems reasonable to me. Could we move it there? Or was there a concern with resizing and the dropdown potentially not fitting in the remaining space there?

  • Are we still planning to do LED patterns or not?

  • It would be nice to align the groups of keys along the same left vertical instead of them being center-aligned. Easier to scan through them. It's fine if we need to change the arrangement of groups to leave less white space on the right. I can try to find a better one. In that regard, how do we figure out what size of the key to use currently?
    image
    image

  • The idea with the designs was that the groups of keys under the keymap are disabled until someone selects a key to remap. It seems like that's not the case now but I'm not entirely sure, I can't check it right now on the actual keyboard. So just want to check how it should work there?

  • I love the new color wheel, looks very neat! We can probably make it smaller, it seems too large right now (at least 1.5 times smaller, but maybe even 2 times). And we should probably give it more padding on the sides (at least 24 px at a glance). For the box with the selected color, we should align its width with the color wheel and use a slight border that is generally used for it everywhere in GNOME. We can probably follow the usual patterns we use in Pop for that one. Or let me know and I'll provide detailed instructions here. Leave more padding at the bottom of the dialog underneath that box.
    image

  • For the color selection dropdown, we can do 4 x 2 grid of colors. We also need to add more padding around them and around the Remove and Edit buttons (at least 12 px). Only in dark mode, the "+" on the button to add a new color is practically invisible right now. I don't know what color we use there now but we should use more contrast.
    Also, originally the idea was to cap the number of colors one can have to 8. I don't know if this has changed.
    image

@ids1024
Copy link
Member

ids1024 commented Jan 14, 2021

Unless we can make the backlight change on the actual keyboard as someone changes the setting in the configurator

Actually that should be quite possible. Indeed, if you try it with actually hardware, you'll find it currently changes the keyboard color while moving around the color wheel. (I thought I might as well make it do that since it can set the color quickly enough for that to be responsive, and it's useful for to see the color on the backlight rather than just an on-screen preview.).

The exact behavior may need some tweaking.

But it seems good to me to try showing the color as a border, and see if we like how that works out.

I still think the "instructions" in the left corner right above the keymap like on the mockup might be somewhat helpful.

I would be somewhat concerned about taking up more vertical space. But moving some of the existing elements should help with that.

The original placement of the dropdown that allows one to quickly switch between layouts was in the left top corner of the header (on the mockups).

The dropdown that's currently displayed switches between multiple keyboards, not layouts. But it's not a good way of handling that; it was just the simplest way to add support for multiple keyboards.

That also brings up the question of could we use shortcuts we planned to use like Ctrl + S for saving and Ctrl + 1, Ctrl + 2, Ctrl + 3, and Ctrl + 4 for switching between 4 layers?

I think if menus are done in the standard way for Gtk, it's easy to add keybindings. It should be possible to support layer switching through the same mechanism.

I can only see two layers at the moment. But I also see three more. I vaguely recall that at least one of them is only there for testing purposes, Electrical if I'm not mistaken. But I can't recall where we have left it with Keycaps and Logical.

@jackpot51 added them. Electrical and Logical don't seem that useful to end users, while Keycaps is. So I've been thinking it could be good to hide those and have a command line argument to show them.

The idea with the designs was that the groups of keys under the keymap are disabled until someone selects a key to remap.

Ah, I'd already changed it to disable that on the keymaps/logical/electrical tabs, but I hadn't though to do so when no key is selected. That should be easy to change.

It would be nice to align the groups of keys along the same left vertical instead of them being center-aligned.

Yeah, it could use some improvement, and the mockup looks better.

I love the new color wheel, looks very neat! We can probably make it smaller, it seems too large right now (at least 1.5 times smaller, but maybe even 2 times). And we should probably give it more padding on the sides (at least 24 px at a glance). For the box with the selected color, we should align its width with the color wheel and use a slight border that is generally used for it everywhere in GNOME. We can probably follow the usual patterns we use in Pop for that one.

Sounds good. I think that should all be fairly straightforward.

For the color selection dropdown, we can do 4 x 2 grid of colors. We also need to add more padding around them and around the Remove and Edit buttons (at least 12 px). Only in dark mode, the "+" on the button to add a new color is practically invisible right now. I don't know what color we use there now but we should use more contrast.
Also, originally the idea was to cap the number of colors one can have to 8. I don't know if this has changed.

Padding sounds good. I guess the color of the + should depend on the theme, given it's background does.

@ids1024
Copy link
Member

ids1024 commented Jan 14, 2021

Regarding the Save/Load buttons, and the layouts dropdown:

Currently Save and Load open a file chooser, and just allow the user to open or save a layout file in any location. Is the goal that the user should instead just enter a name for the layout, and have it saved to a configuration directory (something like ~/.config/system76-keyboard-configuratior/layouts)? What if someone wants to share the layout with another computer/person?

@maria-komarova
Copy link
Author

I would be somewhat concerned about taking up more vertical space. But moving some of the existing elements should help with that.

We can left-align the instruction and right-align the Led controls, that's how it was on the mockups.

The dropdown that's currently displayed switches between multiple keyboards, not layouts. But it's not a good way of handling that; it was just the simplest way to add support for multiple keyboards.

Oh, for that the idea was to have the button with the keyboard icon in the left corner of the header. The icon would take one to the page where you can see whatever keyboards the software can identify. If there are more than two keyboards this should be the first screen to greet the user. Let me know if I should re-post the mockups here for that.

I guess the color of the + should depend on the theme, given it's background does.

Absolutely. Is it actually that color in the theme? Looks barely visible to me.

Currently Save and Load open a file chooser, and just allow the user to open or save a layout file in any location. Is the goal that the user should instead just enter a name for the layout, and have it saved to a configuration directory (something like ~/.config/system76-keyboard-configuratior/layouts)? What if someone wants to share the layout with another computer/person?

Originally I thought that it should save them to the configuration directory. We also planned to have Share button that would be the one to allow the user to share the file either through email or GitHub (need to find the relevant conversation in the history to clarify that). Along with the full-screen (which can be useful but is not a priority). The reasoning was that someone might be playing with the configuration but would want to save it without actually applying to keyboard. Right now there is no Apply changes option which I guess assumes that the changes are saved as they are made?
That would probably work considering we have Load option. I'm wondering though if that is still more convenient if the layouts are right there in the app unless one wants to share.

@maria-komarova
Copy link
Author

Right now the app requires authentication to launch. Is it temporary or the reasoning behind it is because the changes would be written as one tries to use it?

@ids1024
Copy link
Member

ids1024 commented Jan 20, 2021

Is it temporary or the reasoning behind it is because the changes would be written as one tries to use it?

Do you mean in contrast to requesting administrator permission only when one tries to apply changes?

At least with the built-in keyboard, it's necessary to have root access to communicate with the EC, just to read the current layout.

@maria-komarova
Copy link
Author

Do you mean in contrast to requesting administrator permission only when one tries to apply changes?

Yes, that's what I meant.
And I see now. I need to take a closer look at the copy on the authentication dialog then.

@ids1024
Copy link
Member

ids1024 commented Feb 5, 2021

The current version has a smaller color chooser dialog, makes the biding picker insensitive when no key is selected, moves save/load/reset to the menu, adds key bindings for save and load as well as ctrl+number for switching layers. And some other changes.

The mockup has the keybindings listed next to items in the menu, but GtkShortcutMenu doesn't support this, and the convention in Gnome is currently to have a keyboard shortcut dialog. I've added an initial version of this.

Screenshot from 2021-02-05 09-53-11

Of course there are still various things to improve.

Changing border color is fairly easy with how things are currently implemented, but I'm not sure how it looks with some color combinations:

Screenshot from 2021-02-05 09-51-01
Screenshot from 2021-02-05 09-52-00

@ids1024
Copy link
Member

ids1024 commented Feb 10, 2021

@maria-komarova

I have a branch with some WIP code for replacing the keyboard dropdown with a page listing keyboards, similar to the mockup:

Screenshot from 2021-02-10 08-55-03
Screenshot from 2021-02-10 08-55-16

I guess the keyboards should probably be centered, but beside that:

  • In the list of keyboards, what should be displayed on the keys? Keycaps? Layer 1?
  • I included a back button at the top left to get back to the list of keyboards. Is that similar to what you had in mind?
  • It seems a bit awkward having just a small button at the far right to open the keyboard, and then the back button at the opposite side of the Window. (Though part of it is the default window size, which should probably be increased.)

@maria-komarova
Copy link
Author

In the list of keyboards, what should be displayed on the keys? Keycaps? Layer 1?

Probably Layer 1.

I included a back button at the top left to get back to the list of keyboards. Is that similar to what you had in mind?

Yes, it is similar. I was thinking to use the keyboard icon instead of the back button since the idea was to start right with the configuration window if there is only one keyboard detected. I guess back button could work as well, but this is one of the things we can try to test.

It seems a bit awkward having just a small button at the far right to open the keyboard, and then the back button at the opposite side of the Window. (Though part of it is the default window size, which should probably be increased.)

Yes, you are right, it looks awkward. I wonder if centering the button underneath the keyboard might make sense and make things better. I can try to adjust the mockups for that.
We also need to increase spacing between keyboards on this page.

@ids1024
Copy link
Member

ids1024 commented Feb 15, 2021

I was thinking to use the keyboard icon instead of the back button since the idea was to start right with the configuration window if there is only one keyboard detected.

Ah, so that's the reasoning for not using the back icon in the mockup? The potential issue with using the keyboard icon is that it might mean several things. But it probably doesn't matter too much in either case.

@ids1024
Copy link
Member

ids1024 commented Feb 15, 2021

Oh, one notable thing that seems to be absent from the mockups: what should the Configurator show if no supported keyboards are detected?

Currently for debugging it displays a fake launch keyboard, but clearly that's not how the released version should work.

@maria-komarova
Copy link
Author

That's a good reminder. I'll make a mockup for that.

@maria-komarova
Copy link
Author

Just noticed two minor things, but want to drop them here for the record.

  1. I don't see the icon for the configurator in the Launcher.
  2. The bottom border of the selected key in the lower row of the Keyboard is thinner than on the other edges. As if something cuts at it.
    image

@maria-komarova
Copy link
Author

@ids1024 Here is the UI for the LED pattern dropdown, basic list with checkboxes.
conf-layer-layer-pattern

@maria-komarova
Copy link
Author

@ids1024 what do you think about center-aligning everything for the Keyboard selector view?
On the mockup below the button is spaced 2x from the keymap, and then the spacing towards the next header is 4x that (we can try 6-12-48px).
Keyboards

@maria-komarova
Copy link
Author

Sorry, missed the screenshots with the backlight shown as the border. The mockups I did originally had a grey background underneath the keymap to account for lighter colors of the backlight. Like the lighter yellow and white you used in the screenshots. You can see it on one of the mockups above.
But if we can show the backlight change on the actual keyboard, then we can do that instead of using the border.

@ids1024
Copy link
Member

ids1024 commented Feb 19, 2021

On the mockup below the button is spaced 2x from the keymap, and then the spacing towards the next header is 4x that (we can try 6-12-48px).

Hm, that does look better. Not perfect, but it's not like any sane user is likely to have more than two keyboards connected.

But if we can show the backlight change on the actual keyboard, then we can do that instead of using the border.

Okay then, we can do it this way, but changing the border color is still a possibility if we want to do it that way.

If you connect the Launch, turn up the brightness, and open the color picker, you'll see that the color changes immediately as you move around the color wheel.

Control of LED patterns will require firmware support that still needs to be added. But that UI looks reasonable.

@maria-komarova
Copy link
Author

Dropping the basic mockup for "no keyboard detected" state here. I believe an illustration would be good here, keyboard icon is just a placeholder.
Keyboards-none-detected

@maria-komarova
Copy link
Author

@ids1024 one more small thing I just realized, we should ideally have the text on grey and brownish keys on the keymap be white for the sake of hitting the necessary contrast.
image

@ids1024
Copy link
Member

ids1024 commented Feb 23, 2021

This definitely makes things a bit better. I've merged this updated design, so you'll see it if you update the system76-keyboard-configurator branch. I wonder if it would be good to just allow pressing the keyboard, instead of the "Configure Layout" button.

I still need to work on support for dynamically detecting if a keyboard is attached or detached.

Screenshot from 2021-02-23 08-36-48
Screenshot from 2021-02-23 08-43-52

@maria-komarova
Copy link
Author

Definitely looks nicer!

I wonder if it would be good to just allow pressing the keyboard, instead of the "Configure Layout" button.

I still think we need a clear call to action. It might be a bit ambiguous if we don't have one. But I like the idea of making the whole keyboard clickable. That might be a nice addition to the button.

Looking at the spacing, it still feels a bit crammed. We might want to refine the spacing on that view between the keyboard and the button (should be twice what it is on the screenshot above). And therefore, more space to the next header and keyboard.

@ids1024
Copy link
Member

ids1024 commented Mar 15, 2021

The configurator potentially has settings that are:

  • Per-keyboard
    • I believe we said we want brightness to not be layer specific
    • Color settings are not per-layer on the internal keyboard, though maybe a future EC firmware could support it
  • Per-layer
    • Mode, color, brightness
  • Per-key
    • Color when the per-key mode is enabled

How exactly do we want to make clear which settings are per-keyboard, per-layer, or per-key? That doesn't seem entirely clear in the mockups so far.

Another thing to note: I expect many people will want to just use the same settings for every layer. Do we want to have an easy way to do that, without manually changing each?

@maria-komarova
Copy link
Author

Here is the latest mockup to convey that brightness is for all layers.
conf-layer1 (2) (2)
Per-key is available as a custom pattern. I think it is pretty clear that it is per-key.

Color settings are not per-layer on the internal keyboard, though maybe a future EC firmware could support it

What is currently possible to set for the internal keyboard for LEDs? Only colors? Any LED disabling? Any variation within our laptop line?

@ids1024
Copy link
Member

ids1024 commented Mar 15, 2021

Per-key is available as a custom pattern. I think it is pretty clear that it is per-key.

Right. Maybe it could use the same control in the same place, but the label can change to say "Layer LED Color" or "Key LED Color".

What is currently possible to set for the internal keyboard for LEDs? Any LED disabling? Any variation within our laptop line?

I believe the functionality where LED settings are per-layer isn't currently implemented in the EC, but potentially could be.

However, I believe only the bonw14 has per-key LEDs, and we can't really take proper advantage of that since it's controlled by a separate micro-controller running proprietary firmware that couldn't easily be replaced. So the EC firmware allows setting the color for the keyboard overall. Or it only supports brightness for monchrome backlights.

@maria-komarova
Copy link
Author

Right. Maybe it could use the same control in the same place, but the label can change to say "Layer LED Color" or "Key LED Color".

I don't think it should use the same control with a different label. It seems pretty clear to me in the mockups once someone chooses the Custom per-key solid pattern in the dropdown. Then the instruction copy should become more visible and one can either change the color for the whole layer still through the Layer LED color or set them per-key by choosing keys on the keymap.
We might need to test this part. If you think it is confusing I can try to make a larger prototype and test it with a few people. Might take time though.

For the internal keyboards we might have to use something similar to what we did for Brightness.
Keyboard LED color (all layers) and have the color setting.

@maria-komarova
Copy link
Author

I guess it will be brightness and LED color for the internal keyboard. Although I'm not sure we would need a keymap in this case here. There will be no functionality associated with it.

@ids1024
Copy link
Member

ids1024 commented Mar 17, 2021

Getting back to this point:

one more small thing I just realized, we should ideally have the text on grey and brownish keys on the keymap be white for the sake of hitting the necessary contrast

Updated it to check how dark the background is, and use either black or white text depending on that:

Screenshot from 2021-03-17 14-37-10

Also, the LED control is now like this:

Screenshot from 2021-03-17 14-47-34
Screenshot from 2021-03-17 14-48-12

The speed is hidden and the colors are replaced with a saturation slider depending on the mode as described in #27 (comment).

Changes to colors are now saved persistently, every 10s or when the Configurator is Closed. If the keyboard is unplugged or the Configurator crashes less than 10 seconds after color settings are changed, they may revert to their previous value when the keyboard is powered on again.

@maria-komarova
Copy link
Author

That looks great!

I think we can also simplify the color settings by leaving only one color circle, no delete or edit functionality. It makes more sense with the other changes.
We should also update the labels in the listbox to match the latest mockup above.

@maria-komarova
Copy link
Author

The speed is hidden and the colors are replaced with a saturation slider depending on the mode as described in #27 (comment).

For saturation, the description should be something like "Layer LED Color Saturation".

@ids1024
Copy link
Member

ids1024 commented Mar 18, 2021

I think we can also simplify the color settings by leaving only one color circle, no delete or edit functionality.

That does make things simpler, since it's complicated to get the add/remove color functionality working quite as expected.

Screenshot from 2021-03-18 08-04-53
Screenshot from 2021-03-18 08-05-07

@ids1024
Copy link
Member

ids1024 commented Mar 18, 2021

LED keybinds still need to be implemented. There are some complications about how that should work.

The mockup has:

  • Next pattern
  • Prev pattern
  • Led speed +
  • LED speed -
  • LED brighten
  • LED darken
  • LED on/off

If we want all of these settings to be per-layer, other than brightness, how exactly should these work? It can't really just apply to the current layer, since that will just be whichever layer the key is bound to. Not sure how else it could work.

I suppose the other question is what increasing/decreasing speed/brightness once does. Just +1 or -1 is probably too small a change. The internal keyboard is implemented but going through a list of brightness levels. But that's presumably just copying the behavior Clevo implemented. Alternately it may work well to just increase or decrease by a particular increment size.

@ids1024
Copy link
Member

ids1024 commented Mar 22, 2021

I wonder if instead of a disable toggle, there could be an "Off" or "None" mode. Though it would still be necessary to handle disabling a key led in per-key mode differently.

I also wonder if it would be intuitive to have "Transparent" as a mode, like for keybindings, to have the same led settings as the previous layer.

@maria-komarova
Copy link
Author

For the LED controls, I wonder if we need Next pattern and Prev pattern now. That seems like it won't apply now.
Not sure what to do with LED speed increase and decrease. Seems like that is now set in the configurator per layer. So having a physical key control it might be very awkward, you are right. But also if we remove it from the list of option, would it be okay? If we don't remove it, it makes sense that it should be for the entire keyboard but not sure how to indicate it. I'll keep thinking.
For Brightness increments like on internal keyboards seem best. Which would probably need to match the brightness slider in the UI. Like if anyone changes the brightness with the keys, the slider would need to move to that value. Which is not perfect considering that wasn't initiated by the user themselves. But also not matching the slider seems to introduce more inconsistency.

I need to take a look at where we are with disable toggle. I'm not sure I understand how "Off" or "None" mode would be different or better.

I don't think "Transparent" is something that would be clear enough with LEDs. In a way it already happens with patterns. Although we should also decide what happens with LEDs when someone switches to Layer 3 or 4. Should it have the default pattern of Layer1?

@ids1024
Copy link
Member

ids1024 commented Apr 5, 2021

Multiple key selection is now implemented in an initial form, including displaying the colors as borders around keys (which seems kind of necessary for working with per-key colors), and displaying conflicting colors in the color circle as slices.

Currently border color is used to indicate both selection, and color, just with a different width. That should probably be done somewhat differently.

When the color dialog is opened to set the color, it displays the color on the whole keyboard. (Previously in per-key mode, it changed only that key). It would probably be too unresponsive to set half the keys on the keyboard individually each time the color wheel changes. Pressing Save in the dialog will then light up the executed keys, or Cancel reverts to the previous setting.

Screenshot from 2021-04-05 07-35-19
Screenshot from 2021-04-05 07-35-35

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

No branches or pull requests

2 participants