-
Notifications
You must be signed in to change notification settings - Fork 972
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
Revamp of connect and options UI #1410
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks awesome, a definite improvement! Let's do this
vncviewer/vncviewer.cxx
Outdated
#endif | ||
// GNOME uses (53,132,228), macOS (0, 122, 255), this is the lightest | ||
// version of GNOME's that fl_contrast() won't screw up | ||
Fl::set_color(FL_SELECTION_COLOR, 29, 113, 215); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see an easy way to get the borders, and I think it will look odd without those.
Icons won't work as FLTK has no handling for inverting them when things get selected.
I also would have preferred aligning the labels on the left, but it looks horrible as FLTK has basically no margins.
All of this might be possible by doing a custom widget, but that is more work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I love that!
vncviewer/ServerDialog.cxx
Outdated
serverName->align(FL_ALIGN_LEFT | FL_ALIGN_TOP); | ||
y += INPUT_HEIGHT + 20; | ||
|
||
divider = new Fl_Box(20, y, w() - 20*2, 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my eyes, the ServerDialog fits better with the new OptionsDialog if we get rid of this divider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way looks fine to me. Originally I tried to copy a design with a different background for the server name section, to bring attention to that. But I couldn't get that to work, so the divider was a different attempt to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it seems for example my_fl_group->color(FL_BLUE);
does nothing.. Setting a background color works on Fl_Box or Fl_Scroll though.
That looks odd. Looks like the buttons are part of the page, and are not
global for everything.
I agree, I think the buttons look better inside the bottom pane. And this
does look great!
…On Fri, Jan 28, 2022 at 10:21 AM Pierre Ossman (Work account) < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In vncviewer/vncviewer.cxx
<#1410 (comment)>:
> Fl::background(240, 240, 240);
-#endif
+ // GNOME uses (53,132,228), macOS (0, 122, 255), this is the lightest
+ // version of GNOME's that fl_contrast() won't screw up
+ Fl::set_color(FL_SELECTION_COLOR, 29, 113, 215);
That looks odd. Looks like the buttons are part of the page, and are not
global for everything.
But fair enough, if you want it that badly. ;)
Here's what a more complete implementation would look like:
[image: Screenshot from 2022-01-28 16-19-15]
<https://user-images.githubusercontent.com/6640916/151572986-e09a512d-9bd7-4838-913e-488910f0fbb9.png>
—
Reply to this email directly, view it on GitHub
<#1410 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB45M3PVTALZUVOAIYIEZE3UYKX6RANCNFSM5MBAVLAA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
That is sweet! I love the background and the fact that the version is
visible. I also prefer the wider aspect ratio because the hostname is less
likely to be truncated
…On Fri, Jan 28, 2022 at 11:40 AM Pierre Ossman (Work account) < ***@***.***> wrote:
I'm not entirely happy with the server dialog. Another approach is this:
[image: Screenshot from 2022-01-28 17-39-49]
<https://user-images.githubusercontent.com/6640916/151586411-b8476802-7eb9-424f-b3c5-d847354d057b.png>
—
Reply to this email directly, view it on GitHub
<#1410 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB45M3NPEN2FEY6INI4JV73UYLBILANCNFSM5MBAVLAA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Was it something like this you imagined with regards to a different color to highlight the server name section @CendioOssman? I realize this is highly subjective, but I would prefer not to add too much color. |
I like that better than the vertical layout but I still think the extra
graphics looked fantastic
…On Wed, Feb 2, 2022 at 9:54 AM Pierre Ossman (Work account) < ***@***.***> wrote:
Another approach that avoids too much graphics, yet still gets the
horizontally dominant layout:
[image: Screenshot from 2022-02-02 15-52-53]
<https://user-images.githubusercontent.com/6640916/152177757-eb50ee76-9df0-43b4-a62d-c84ff9a96558.png>
—
Reply to this email directly, view it on GitHub
<#1410 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB45M3KF7IRQJ7JISWZAQE3UZFARRANCNFSM5MBAVLAA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
I like this last one the best 🙂 Is something stopping us from merging this? I think the changes are logical and things look great. |
I'd like to play around with it a bit more first and see how far I can push this. Both Windows and GNOME are moving towards a more flat and rounded look, so I'd like to see how close we can get. Prototype looks like this: The problem is that FLTK is not really flexible enough to do this. So we might have to reimplement convenience functions such as |
I also need to check how this fits with Windows and macOS. I've just been comparing with GNOME so far. |
Wow, the latest screenshot looks great! This is going to be awesome Is it possible to change the look of the file chooser / ask dialog at all without making our own? |
The ask dialog is likely easier to just do our own. Haven't checked the file chooser. The built-in one is rather horrible anyway, so might be an excuse to do a better one. |
Any chance of a dark mode? 😀 |
Probably not. There is enough feature creep on this as it is. So dark mode is best dealt with separately. |
We should have a consistent color set over all widgets.
Modern UIs use lighter colors. Let's fit in. Mostly taken from GNOME's default theme.
Modern systems generally use 11pt rather than the 10pt we're targeting. Increase this to almost 11pt to fit better in. Going further looks a bit odd because of all hard coded margins in FLTK.
Try to make things look a little bit more appealing, and reduce some of the clutter at the same time with a focus on the commonly used elements.
Try to match GNOME's default theme a bit better.
To fit better with what Windows itself uses.
To better match what native widgets look like. This also adds a shaded edge on round boxes, even though that doesn't match Windows and macOS. However, we cannot properly match what they do for radio buttons, so let's be internally consistent instead.
They have flat boxes instead. They also use white, or very light, colors for interactive elements. Unfortunately we can't directly control the colors of widgets, so instead we just lighten everything that uses this box type.
These makes them look less harsh, especially on platforms without anti-aliasing. This is also what both macOS and GNOME do. For up boxes we draw outside the given boundaries. Hopefully this won't cause side effects...
Follow the colors that modern desktops use when it comes to background, text and selections.
In old connect window, there are also "Load" and "Save as" buttons. I can't find anything like in the new UI (also not within the Options window...). Does it mean we won't get those possibilities with the new UI? I think it would be great to keep a simple way to export and import current settings... |
I was hoping to move those functions in to the options dialog, yes. |
This new UI is much better. |
I really don't like the vertical versions of the branding. In fact, I love that the current version has none. I'm using a VNC viewer, so I really don't think there's any need to inspire brand loyalty, so when I have the connection window on my screen, I don't want it to waste space. The titlebar already tells me what application I'm using, and if the I think that the slight horizontal padding on the selected option really helps since you haven't put any button outlines. I find that myself (despite being young) and some older folk do not necessarily notice that those kinds of options are buttons. Although a vertical list is a great improvement over the current horizontal navigation tabs, they're less obviously buttons in most implementations. This assists vaguely. |
That's a good point, I agree. |
I'd always prefer a design like GitHub/KDE's, where anything clickable, such as the buttons, are always outlined, but I understand that that might not fit with the GNOME HIG, if that's what you're intending to replicate. |
A suggestion of revamping the UI a bit. The main driving factor was that the tab layout in the options dialog was limiting future development. But as I was poking around with that I did some general cleanup as well to make things slightly less dated.
Before:
After: