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

Change to modern layout in options dialog #1529

Merged
merged 14 commits into from
Dec 22, 2022

Conversation

CendioOssman
Copy link
Member

Classical tabs are very dated. They are also a practical problem as you get very limited in the numbers of tabs we can have, and how long the text can be on them.

Let's separate TigerVNC specific things from stuff that could be part of
upstream FLTK. These are files that we would like to collaborate with
other FLTK users, so they are more liberally licensed and avoid using
TigerVNC specific things.
These have nothing to do with layout, so let's split them to their own
file.
These are general things and not specific to TigerVNC, so let's move it
to the fltk specific directory for clarity.
Follow upstream FLTK naming to more clearly indicate that this is a
general widget and not TigerVNC specific.
The accessor functions are called value() for all standard FLTK widgets,
so let's keep that theme here as well.
@CendioOssman CendioOssman added the enhancement New feature or request label Sep 7, 2022
@CendioOssman
Copy link
Member Author

Before:

Screenshot from 2022-09-07 14-45-30

After:

Screenshot from 2022-09-07 14-45-21

@CendioOssman
Copy link
Member Author

This is broken out of #1410 so that it can be merged and used for #1517.

@dmcmahill
Copy link

Thanks for doing this. The new dialog looks much nicer.

@dmcmahill
Copy link

I have reworked the code from #1517 on a branch that is based on the branch in this PR. Once this PR merges, I'll rebase my new version and update #1517

Copy link
Member

@samhed samhed left a comment

Choose a reason for hiding this comment

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

I have tested it on Fedora 37, Windows 11 and macOS 13, it works well and looks nice. I also looked through the code and don't have any complaints.

Just two questions really:

  • The naming of "Notebook"
  • See the below screenshot, do we want some sort of delimiter between the content and the window titlebar? To my eyes it looks a bit odd and "cut off" with Windows 11's white color.

newoptions_win11


// Odd dimensions to get rid of extra borders
Fl_Notebook *notebook = new Fl_Notebook(-1, -1, w()+2,
h()+1 - INNER_MARGIN - BUTTON_HEIGHT - OUTER_MARGIN);
Copy link
Member

Choose a reason for hiding this comment

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

I'm unsure about the metaphor in this name. In what way does a notebook resemble something with a sidebar for navigation?

Looking at GTK's naming, see the "StackSidebar" widget in their gallery here, I think that one is very similar:

https://docs.gtk.org/gtk4/visual_index.html#containers

Copy link
Member Author

Choose a reason for hiding this comment

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

It was named after GtkNotebook, which was the Gtk widget you classically had to use to build something like this. It is equivalent to Fl_Tabs, though.

A GtkStackSidebar seems to just be the left list. And a GtkStack seems to widget handling the content.

Qt seems to have a similar setup, where you create a QStackedWidget and pair it with QListWidget.

So no direct equivalent in the FOSS world.

WinUI has something very similar, though, which they call NavigationView.

I don't have any particular strong preferences for naming here.

Copy link
Member

Choose a reason for hiding this comment

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

I see. I like NavigationView, but Notebook is fine too.

@CendioOssman
Copy link
Member Author

See the below screenshot, do we want some sort of delimiter between the content and the window titlebar? To my eyes it looks a bit odd and "cut off" with Windows 11's white color.

Yeah, that doesn't look nice. I'll have a look.

@CendioOssman
Copy link
Member Author

It would look like this instead:

Skärmbild 2022-12-21 140039

I only changed things on Windows, as it looked odd to increase the border on the other platforms.

@samhed
Copy link
Member

samhed commented Dec 22, 2022

Nice, that looks better!

Classical tabs are very dated. They are also a practical problem as you
get very limited in the numbers of tabs we can have, and how long the
text can be on them.

Switch to one popular modern model with a list on the left instead where
pages can be selected.
Outlines are no longer commonly used. Instead visually separated
headlines are the norm.
Makes it easier to adjust the UI from a central place.
Makes it easier to adjust the UI from a central place.
This makes the code much cleaner, and easier to update.
To conform with how e.g. GNOME sets its margins.
This is how GNOME does things, so let's do the same so things look
similar.
Try to follow the actual padding that FLTK adds to these widgets.

The extra one pixel on each is because of a bug in FLTK's focus drawing
code, where the box is always one pixel too small in both dimensions.
Use the font specified by the system for UI elements. For Windows and
macOS this is straight forward, but Linux is more complex as there is no
single source for this information.
@CendioOssman CendioOssman merged commit 1d82602 into TigerVNC:master Dec 22, 2022
@CendioOssman CendioOssman deleted the newoptions branch February 23, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants