Opened 8 years ago

Closed 8 years ago

#5190 closed Enhancement (fixed)

Add borders instead of shadow on scrolled window

Reported by: nagisa Owned by: jordan
Priority: Normal Milestone: 2.80
Component: GTK+ Client Version: 2.75
Severity: Minor Keywords:
Cc:

Description

A purely cosmetic change proposal:

Instead of adding a shadow with gtk_scrolled_window_set_shadow_type(GTK_SCROLLED_WINDOW (w), GTK_SHADOW_IN); in gtk/tr-window.c, I propose using CSS styling available from GTK+3 and adding borders on scrolled window using following styles:

border-style: solid;
border-size: 1px 0;

Comparison screen shots of how transmission looks now (note the ugly double border on both sides) and how transmission should look after proposed are attached.

Attachments (8)

18.png (17.8 KB) - added by nagisa 8 years ago.
Current look
18_borders.png (17.7 KB) - added by nagisa 8 years ago.
Look after proposed change
gtk-client-with-shadows.png (46.4 KB) - added by jordan 8 years ago.
current behavior behavior (shadows in the scrolled window)
gtk-client-with-separators.png (46.6 KB) - added by jordan 8 years ago.
behavior in ticket-5190-rev-001.diff: remove the shadows, add horizontal separators between the torrent list and the widgets above/below
ticket-5190-rev-001.diff (1.1 KB) - added by jordan 8 years ago.
possible fix
20.png (5.8 KB) - added by nagisa 8 years ago.
Glade mockup with horizontal separators in adwaita. Note that separator above white area is barely visible.
glade_mockup.glade (11.0 KB) - added by nagisa 8 years ago.
Glade mockup made to produce earlier screenshot.
tr-5190.patch (2.4 KB) - added by nagisa 8 years ago.
A patch

Download all attachments as: .zip

Change History (18)

Changed 8 years ago by nagisa

Current look

Changed 8 years ago by nagisa

Look after proposed change

Changed 8 years ago by jordan

current behavior behavior (shadows in the scrolled window)

Changed 8 years ago by jordan

behavior in ticket-5190-rev-001.diff: remove the shadows, add horizontal separators between the torrent list and the widgets above/below

Changed 8 years ago by jordan

possible fix

comment:1 Changed 8 years ago by jordan

I think we can fix this by removing the shadows altogether and adding horizontal separators between the torrent list and the widgets above & below it. The result looks the same and is a cleaner approach than changing the CSS.

nagisa, any opinion on this?

comment:2 Changed 8 years ago by jordan

  • Milestone changed from None Set to 2.80
  • Severity changed from Normal to Minor

Changed 8 years ago by nagisa

Glade mockup with horizontal separators in adwaita. Note that separator above white area is barely visible.

Changed 8 years ago by nagisa

Glade mockup made to produce earlier screenshot.

comment:3 Changed 8 years ago by nagisa

Yes, that's another solution. Result looks good, but I disagree that it does look the same as with CSS – separator styling depends on theme and for example in Adwaita separator is much lighter than regular borders. I suspect this may be a case for several other themes as well.

comment:4 Changed 8 years ago by jordan

Okay, TBH this is my first exposure to mixing CSS rules into GTK at the application level. Do you want to cook up a patch for how to do this in the code?

comment:5 Changed 8 years ago by nagisa

Sure. I'll just need to figure where to put a stylesheet. You don't seem to have any other data which would require to creating a separate data directory (like /usr/share/transmission/) for it yet.

Making custom stylesheets work is mostly just gtk_style_context_add_provider() away for a single widget or gtk_style_context_add_provider_for_screen() away for all widgets.

Changed 8 years ago by nagisa

A patch

comment:6 Changed 8 years ago by nagisa

Apparently after making a patch I realized I've made a mistake while filling this bug. It is impossible to add border without having a shadow on.

After that it's just shadow styling, which is mostly just hiding a shadow on both sides and leaving it intact on the top and the bottom.

comment:7 Changed 8 years ago by jordan

nagisa, does the patch need to be updated for the problem you described in comment:6?

comment:8 Changed 8 years ago by nagisa

jordan, no, patch already accounts for it and doesn't remove the shadow, just adds styling to it. It can be merged.

comment:9 Changed 8 years ago by jordan

  • Status changed from new to assigned

comment:10 Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

Cool, thanks.

r13822: (gtk) #5190 'Add borders instead of shadow on scrolled window': fixed, patch by nagisa

Note: See TracTickets for help on using tickets.