Skip to content

Fix FileSystem dock styles across split modes#119450

Open
norepro wants to merge 2 commits into
godotengine:masterfrom
norepro:filesystem-split-styles
Open

Fix FileSystem dock styles across split modes#119450
norepro wants to merge 2 commits into
godotengine:masterfrom
norepro:filesystem-split-styles

Conversation

@norepro
Copy link
Copy Markdown
Contributor

@norepro norepro commented May 14, 2026

Two commits for scroll hints had impact on FileSystem dock style:

  • f187b8b introduced TreeSecondary for the tree pane, but only applied it in horizontal split mode.
  • a45bdce removed the existing ItemListSecondary from the file list in vertical split mode.

The combination results in vertical split and tree-only not having background styles while horizontal split does.

Apply TreeSecondary and ItemListSecondary styles in all split modes. This gives consistency between the modes and with other elements.

Fixes #119359

Two commits for scroll hints had impact on FileSystem dock style:

- f187b8b introduced `TreeSecondary` for the tree pane, but only
applied it in horizontal split mode.

- a45bdce removed the existing `ItemListSecondary` from the file list
in vertical split mode.

The combination results in vertical split and tree-only not having
background styles while horizontal split do.

Apply TreeSecondary and `ItemListSecondary` styles in all split modes.
This gives consistency between the modes and with other elements.

Fixes godotengine#119359
@norepro norepro requested a review from a team as a code owner May 14, 2026 05:03
Copy link
Copy Markdown
Member

@YeldhamDev YeldhamDev left a comment

Choose a reason for hiding this comment

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

There are two severe problems here:

  • The single-view mode having a background breaks visual consistency with the rest of the docks.
  • The horizontal split-view mode backgrounds have no margins, so they just hug the borders.

@AdriaandeJongh
Copy link
Copy Markdown
Contributor

So the solution would be to just remove the backgrounds everywhere instead, right?

@YeldhamDev
Copy link
Copy Markdown
Member

YeldhamDev commented May 14, 2026

So the solution would be to just remove the backgrounds everywhere instead, right?

I'm not sure I follow? Backgrounds are hidden when scroll hints can be used. There are plenty of places where backgrounds are still used where hints don't make sense.

@AdriaandeJongh
Copy link
Copy Markdown
Contributor

Like the author, I’m trying to figure out a solution to fix the inconsistency that in only one of the three modes the background appears. What do you think the solution is?

@norepro
Copy link
Copy Markdown
Contributor Author

norepro commented May 14, 2026

There are two severe problems here:

* The single-view mode having a background breaks visual consistency with the rest of the docks.

* The horizontal split-view mode backgrounds have no margins, so they just hug the borders.

Fair. This issue is more complicated than I thought... my goal was to make it self-consistent among split modes. But I am guessing way more users see the default single-view than split and,by comparison, it is the odd one out.

I also see the second point. I was focused more on the colors and usability and missed that detail.

I'll mark this PR as draft while I consider options here and bring up any ideas in the discussion thread.

@norepro norepro marked this pull request as draft May 14, 2026 06:16
@AThousandShips AThousandShips added this to the 4.x milestone May 14, 2026
- Do not apply style to single-view mode
- Add margins to `HSPLIT` and `VSPLIT` modes
- Remove scroll hints from both split views because they should not
exist with backgrounds.
@norepro norepro marked this pull request as ready for review May 16, 2026 04:53
@norepro norepro requested a review from YeldhamDev May 16, 2026 04:53
@YeldhamDev
Copy link
Copy Markdown
Member

YeldhamDev commented May 16, 2026

CC @passivestar, since this is a design change, even if small.

Copy link
Copy Markdown
Contributor

@passivestar passivestar left a comment

Choose a reason for hiding this comment

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

This shifts it from "in 1 of 3 modes the background appears" to "in 2 of 3 modes the background appears", if @AdriaandeJongh is satisfied with it I'm fine with it, since it's his original linked issue. I think it's fine if the goal is to get the same look for both split modes.

If I could I'd make a rulebook for when there needs to be a background and when not, but I looked at a lot of different apps when we were porting the theme to see how others handle it, and it's all case by case depending on where it makes sense, with most leaning towards no backgrounds unless there's a big benefit from them. One thing I'm sure of is that you don't want to go for full consistency here because the previous theme consistently had backgrounds in every tree and every item list and it made the UI look busier than it should have been in the main editor layout.

@YeldhamDev YeldhamDev modified the milestones: 4.x, 4.7 May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FileSystem panel: vertical split mode & non-split mode lack background

5 participants