After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 736408 - find popup is 'floating'
find popup is 'floating'
Status: RESOLVED FIXED
Product: yelp
Classification: Applications
Component: General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Yelp maintainers
Yelp maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-10 11:18 UTC by Matthias Clasen
Modified: 2014-09-11 15:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add some styling to the find revealer (2.70 KB, patch)
2014-09-10 16:40 UTC, David King
committed Details | Review
floating find entry (75.28 KB, image/png)
2014-09-10 16:52 UTC, David King
  Details
find entry with styling fixes in patch (75.19 KB, image/png)
2014-09-10 16:53 UTC, David King
  Details
yelp and gedit find bars (920.69 KB, image/png)
2014-09-10 21:59 UTC, David King
  Details

Description Matthias Clasen 2014-09-10 11:18:30 UTC
it just floats over content, which looks irritating. Compare to gedit, which has a very similar popup, but it has a white box around it that 'anchors' it to the top.
Comment 1 David King 2014-09-10 16:40:29 UTC
Created attachment 285837 [details] [review]
add some styling to the find revealer

The attached path styles the find revealer in the same way as done in gedit. It just uses some in-line CSS, as I could not find any other CSS in Yelp to add the overrides to.
Comment 2 David King 2014-09-10 16:52:30 UTC
Created attachment 285839 [details]
floating find entry
Comment 3 David King 2014-09-10 16:53:04 UTC
Created attachment 285840 [details]
find entry with styling fixes in patch
Comment 4 Shaun McCance 2014-09-10 17:29:20 UTC
Yeah, now I remember not finishing this. I think I remember digging through Adwaita's CSS looking for a style class to use and couldn't find one. I'm surprised I need to add custom CSS for this.
Comment 5 Shaun McCance 2014-09-10 21:18:28 UTC
Border color is kind of dark. Darker than gedit's.
Comment 6 David King 2014-09-10 21:59:51 UTC
Created attachment 285867 [details]
yelp and gedit find bars

I do not think that it is too surprising that custom CSS is required, as the revealer is only for animating the display of a widget. It does not draw a background, and neither does a box, which is what the entry and buttons were packed into. If you remove the CSS, and are left with the GtkFrame, the result is pretty ugly.

The CSS was copied from gedit (master, of course, so you might not be familiar with all the Adwaita changes that have happened recently). :-)
Comment 7 Shaun McCance 2014-09-11 00:18:37 UTC
If it's a design pattern we expect developers to use, then I would expect a common CSS class. We have plenty of classes we use to style otherwise-generic widgets to create looks we want.

Your gedit looks different than mine, but I'm not running gedit from master. If that's what it looks like now, then I guess they match.
Comment 8 David King 2014-09-11 08:22:29 UTC
I think that the expected widget for this design pattern would be GtkSearchBar. Yelp already uses that, for a different purpose (searching for a document rather than within a document), and the same widget is also used by other applications such as Epiphany, Logs, Boxes and so on. There are a few applications, such as Evince, that have a custom (but similar-looking) widget to accomplish a similar task. Although Yelp does not quite fit into the typical GNOME 3 application mould, the HIG has a page on search:

https://git.gnome.org/browse/gnome-devel-docs/tree/hig/C/search.page

which suggests a similar approach (search bar sliding in from under the header bar).

I think that it is reasonable to present Yelp's search and find functionality differently, and it seems natural to try to match the styling to that of another GNOME application such as gedit. A new widget or style class could come later, if enough applications want to follow the same design pattern, but it seems that one search pattern is enough to cover the majority of cases.

Anyway, not to push too much, but the patch will need a UI freeze break exception, so it would be good to get a yay/nay and then (hopefully) proceed to get the necessary release team approvals before Monday, in time for the 3.13.92 release.
Comment 9 Shaun McCance 2014-09-11 13:27:05 UTC
Oh, I'm fine with the patch. I was just making general commentary on why it's in the state it is. I didn't realize we're already in UI freeze.
Comment 10 David King 2014-09-11 15:51:20 UTC
Comment on attachment 285837 [details] [review]
add some styling to the find revealer

Pushed to master with release team approval:

https://mail.gnome.org/archives/release-team/2014-September/msg00048.html