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

Remove padding and box around the console #1976

Merged
merged 1 commit into from
Mar 25, 2025
Merged

Remove padding and box around the console #1976

merged 1 commit into from
Mar 25, 2025

Conversation

lawik
Copy link
Contributor

@lawik lawik commented Mar 23, 2025

I think this is a better immersive console.

Next step in my book is a theatre-mode, faux-fullscreen. But this is a good start on using the space.

From this:
Screenshot 2025-03-23 at 09 49 44

To this:
Screenshot 2025-03-23 at 09 49 28

@lawik lawik requested review from joshk and nshoes March 23, 2025 08:52
@lawik
Copy link
Contributor Author

lawik commented Mar 23, 2025

I guess I need to look at where to put presence-bubbles? Or are we putting them outside this area?

@fhunleth
Copy link
Contributor

Thanks for making more space!!!!

Regarding presence bubbles, how about to the left of version and firmware enabled? The reason being that no matter what tab I'm on, I want to know if someone else is using the device.

@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

I think that's been up as a suggestion. I think we technically have both the case of "who is looking at the device" and "who is in the console" to consider. @elinol @joshk you both know more about this than I.

@joshk
Copy link
Collaborator

joshk commented Mar 24, 2025

I'm torn on this change. I find the removal of all spacing a bit hard on my eyes, and inconsistent with the styling we have bought in across the platform.

I'd personally prefer to keep the old style, reduce the spacing a little within the box that houses the terminal, and implement 'theater mode'

@elinol
Copy link
Contributor

elinol commented Mar 24, 2025

Regarding the placement of presence bubbles: in the design documents they are placed in the top bar, left to the pin/reboot.. buttons.

@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

I will try some more experiments with using the box. I suppose theatre mode is what we can currently agree on.

I consider the terminal conceptually similar to a map or video where you want to remove as much UI surrounding it as possible and make it "immersive" in the mobile-app parlance. If the box was providing structure to a few different element it would be a different matter. To me it just looks like we have the box because we use boxes in this UI. It doesn't look intentional because I don't see it provide function, but cost real-estate.

Now I did give it some spacing from the edges but not very much and that might also improve things. I will do a few more experiments for consideration and then try theatre mode :)

@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

Screenshot_20250324_095840
more padding?

@lawik lawik force-pushed the max-size-console branch from f310d8b to 6bd2699 Compare March 24, 2025 09:08
@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

Screenshot_20250324_100956

This one is very close to the original saves no space, just makes the box nicer in my eyes.

@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

Screenshot_20250324_101136

This is with the header removed since I find the header redundant on this view.

If the box provides some helpful structure, then this I suppose still does that but I'm not convinced :)

@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

Side-by-side:

Screenshot_20250324_095840
Screenshot_20250324_100956
Screenshot_20250324_101136

@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

Note that the scrollbar display is different between my Mac and Linux machine :D

@lawik
Copy link
Contributor Author

lawik commented Mar 24, 2025

Screenshot_20250324_132858

I have a theater mode version. But the code needs to change a bit depending on which path we take with this. Will ship a branch for the basic one :)

@nshoes
Copy link
Contributor

nshoes commented Mar 24, 2025

I'm torn on this change

FWIW, I am not. I don't mind which way we go:
1.) Save on space and keep the UI consistent between tabs + theatre mode
2.) Make the console fill out the space more by default + theatre mode

I like the feel-good consistency of spacing and layout between tabs, but I think the console is an exception. I would vote function over form if it comes down to it, and that doesn't detract from the device UI as a whole.

I don't see it provide function, but cost real-estate.

Fully agreed.

@fhunleth do you feel strongly either way? I could see some power users not caring what the default UI looks like if you can use theatre mode.

@joshk
Copy link
Collaborator

joshk commented Mar 24, 2025

I'm 👍 on your 'more padding?' option, which does address my primary concern.

And I love theatre mode, so those two together make a great team.

One thing to note, we are losing the 'console version' from the header of the box. That isn't a bad thing in my eyes as that info wasn't really useful.

@lawik
Copy link
Contributor Author

lawik commented Mar 25, 2025

I'll wrap up the padded immersive option then because I think that's the best one :)

I think this is a better immersive console.
@lawik lawik force-pushed the max-size-console branch from 6bd2699 to e26b6d3 Compare March 25, 2025 07:49
@lawik
Copy link
Contributor Author

lawik commented Mar 25, 2025

This is the edge-to-edge padded version. I also tightened up the error variants. They now use the same "immersive" style and monospaced font to feel a bit console-ish.

Removed a lot of boiler-template-plate.

Copy link
Collaborator

@joshk joshk left a comment

Choose a reason for hiding this comment

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

Although I haven't checked out your code, I trust it :)

@lawik lawik merged commit 5654e88 into main Mar 25, 2025
2 checks passed
@lawik lawik deleted the max-size-console branch March 25, 2025 08:52
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.

5 participants