Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#3936 closed defect (fixed)

Grey rounded rect fill style of gtk3 Frame and Activity toolbar active and pressed icons too large

Reported by: garycmartin Owned by: manuq
Priority: High Milestone:
Component: Sugar Version: 0.97.x
Severity: Major Keywords: 13.1.0, theme, regression, r+, olpc-test-passed
Cc: manuq, humitos, erikos Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

The gtk3 grey rounded rect background fill style of the Frame and Activity toolbar active or pressed (mouse down / click) icons are too large. The grey background rounded rect should be smaller than the full hight of the frame/toolbar. See attached screen shot for before & after comparison.

Attachments (7)

gtk3-vs-gtk2-frame-and-toolbar-active-icon-style-too-large.png (45.6 KB) - added by garycmartin 7 years ago.
margin-on-frame-toolbutton.diff (469 bytes) - added by humitos 7 years ago.
frame-size_border-width-3px.jpeg (20.7 KB) - added by humitos 7 years ago.
The result after applying the patch
margin-on-frame-toolbutton-v2.diff (635 bytes) - added by manuq 7 years ago.
Follow up of humitos patch.
Icon patch padding test animation.gif (125.1 KB) - added by garycmartin 7 years ago.
Gif animation of frame before patch, after patch, and an old 0.94.1 build for reference
0001-Get-back-margin-of-toolbuttons-SL-3936.patch (1.6 KB) - added by manuq 7 years ago.
Candidate patch.
0002-RadioToolButton-draw-the-outline-properly-SL.patch (1.8 KB) - added by manuq 7 years ago.
Toolkit patch to get the outline right. Was affecting only RadioToolButtons like the ones in the top panel of the frame, or in Paint toolbar.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 7 years ago by manuq

  • Owner changed from erikos to manuq
  • Status changed from new to assigned

Yes, I fighted this style difference at more than one point of the new theme development. Supposedly all widgets have settable margin, but it has no effect in toolbuttons.

.toolbar .button {
    margin: 3px;
}

doesn't work.

comment:2 Changed 7 years ago by manuq

  • Keywords theme added

comment:3 Changed 7 years ago by manuq

  • Priority changed from Unspecified by Maintainer to High
  • Severity changed from Unspecified to Major

comment:4 Changed 7 years ago by humitos

  • Cc humitos added

We need to use background-clip property to cut the background at some point.

I'm attaching a patch that uses it and makes the "active" icons to look similar they were in gtk2. Although, this patch needs a bit more of work because it seems that F* icons have a fixed width and they do not keep with the actual value. Besides, the hight of the frame should be increased 6px as well because we need 3px for padding (at the top and at the bottom = 6px)

Is it possible to increase those values from the design point of view?

Changed 7 years ago by humitos

Changed 7 years ago by humitos

The result after applying the patch

Changed 7 years ago by manuq

Follow up of humitos patch.

comment:5 Changed 7 years ago by manuq

Great! My follow up patch removes some padding from the button to keep the toolbar height. This needs to be well tested and proven with screenshots.

comment:6 follow-up: Changed 7 years ago by manuq

  • Cc erikos added
  • Keywords r? added

Candidate patch attached. I have verified this has the same margin as the GTK2 theme. Two things worth mention:

  1. as we are using a transparent border to get the margin, there is one state we can't get fully correct: an active button wich is being hovered. It should be a black fill below a light grey rounded button.
  1. the code that draws an outline around the button when it has a palette, should be updated to consider the margin.

comment:7 in reply to: ↑ 6 Changed 7 years ago by garycmartin

Replying to manuq:

Candidate patch attached. I have verified this has the same margin as the GTK2 theme. Two things worth mention:

  1. as we are using a transparent border to get the margin, there is one state we can't get fully correct: an active button wich is being hovered. It should be a black fill below a light grey rounded button.
  1. the code that draws an outline around the button when it has a palette, should be updated to consider the margin.

Yes, I see both these issues as well, but otherwise I've not yet notices any other side effects in the Frame or Activities. Interestingly I could only get this to make a visible difference on an XO-1.75, it applied correctly to an XO-4 without error but has no visible difference – still investigating, must be something I'm doing (both are running 13.1.0 build 16 with fresh git make/installs of toolkit, shell, artwork, plus this artwork patch and nothing else).

comment:8 Changed 7 years ago by manuq

  • Keywords regression added

Changed 7 years ago by garycmartin

Gif animation of frame before patch, after patch, and an old 0.94.1 build for reference

comment:9 Changed 7 years ago by garycmartin

Just added an animated gif animation of the frame before the patch, after the patch, and an old 0.94.1 build just for reference. There is some notable horizontal padding related movement with the new patch.

Changed 7 years ago by manuq

Candidate patch.

comment:10 Changed 7 years ago by manuq

Great test Gary! I have updated the patch based on your findings, just use the same horizontal padding tnan vertical one. Can you try again?

Changed 7 years ago by manuq

Toolkit patch to get the outline right. Was affecting only RadioToolButtons like the ones in the top panel of the frame, or in Paint toolbar.

comment:11 Changed 7 years ago by manuq

  • Keywords r+ olpc-test-pending added; r? removed
  • Resolution set to fixed
  • Status changed from assigned to closed

Pushed artwork 7a0b1b51 and toolkit 2a6ad05b .

comment:12 Changed 7 years ago by greenfeld

  • Keywords olpc-test-passed added; olpc-test-pending removed

The grey highlight in toolbars does not extend to the full height of the frame (as desired) in Sugar 0.98.2 or .3.

comment:13 Changed 6 years ago by dnarvaez

  • Component changed from sugar-artwork to Sugar

comment:14 Changed 6 years ago by dnarvaez

  • Milestone 0.98 deleted

Milestone 0.98 deleted

Note: See TracTickets for help on using tickets.