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 761240 - unify max-width for all views
unify max-width for all views
Status: RESOLVED FIXED
Product: gnome-software
Classification: Applications
Component: General
3.19.x
Other Linux
: Normal normal
: ---
Assigned To: GNOME Software maintainer(s)
GNOME Software maintainer(s)
Depends on:
Blocks: 735994
 
 
Reported: 2016-01-28 13:17 UTC by Jakub Steiner
Modified: 2017-02-17 11:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Applies max-width=860 to the app details page (131.22 KB, patch)
2016-02-11 00:26 UTC, Rafal Luzynski
none Details | Review
Applies max-width=860 to the app details page (V2) (115.88 KB, patch)
2016-04-01 10:33 UTC, Rafal Luzynski
none Details | Review
what i see (17.98 KB, image/png)
2016-04-01 10:45 UTC, Richard Hughes
  Details
Example of the expanded main screenshot widget (315.26 KB, image/png)
2016-04-08 21:31 UTC, Rafal Luzynski
  Details
Expand the width of the main screenshot background (1.09 KB, patch)
2016-04-18 21:01 UTC, Rafal Luzynski
committed Details | Review
Applies max-width=860 to the app details page (V3) (124.36 KB, patch)
2016-04-25 08:47 UTC, Rafal Luzynski
committed Details | Review

Description Jakub Steiner 2016-01-28 13:17:46 UTC
We have a nice way to deal with super large screen sizes now, having a max-width for legible line length. However the front page allows for a much wider width and switching views makes the varying widths look unpolished.

I think we should find a good max-width size that can be applied for all views.
Comment 1 Rafal Luzynski 2016-01-29 00:08:46 UTC
This feature is already implemented for Installed, Updates, Extras and Search results, the maximum width is 860px - see bug 758662.

Details page - this feature would be highly desirable although I'm not sure that 860px is the best value. Currently there is a similar feature: the width_request property is set to 752px. Unfortunately, width_request is not a good choice because in fact it is the minimum width rather than maximum. Sometimes the window may be wider than 752px and it leads to bad results but never smaller. This is being reworked as bug 735994.

Category page - since bug 758669 is fixed the maximum width of the right panel only is set to 1338px. If it was smaller this would undo the effect of responsive scaling. The right panel must be at least 888px wide to switch to 3 columns. Note that it does not count the left panel (the list of subcategories), spacing, margins etc.

Overview page - setting the maximum width to 860px would undo the effect of fixing the bug 757986, the situation would be even worse because you can fit only 5 app tiles at 860px (the default width of an app tile is 152px). The fix for bug 757986 increases the maximum number of app tiles from 6 to 8.

I understand that a common fixed width for all pages would look good but it may be difficult or even impossible to achieve unless you sacrifice some of the most recent changes.
Comment 2 Allan Day 2016-02-04 11:30:46 UTC
I wouldn't worry about the categories view here: that's got the sidebar and the content isn't centered, so it's a bit different. The relevant pages for me are:

 * Overview page
 * App details pages
 * List pages (installed, updates, search results)

The details and list pages seem a bit on the narrow side for me, and look like they could be expanded. 

I really like how the overview page expands to make use of available space. However, its maximimum width (looks about 1314px to me) might be too wide for the details pages.

I see two options therefore:

 1. Use a consistent width of around 1200px for all the relevant pages - this would reduce the overview page a bit
 2. Stick with the current width for the overview page, but try and make the details and list pages a bit closer: we could expand them to around 1100px wide or so
Comment 3 Rafal Luzynski 2016-02-05 10:25:52 UTC
Thanks for answering, Allan. So again:

* Categories view: will be left unchanged.
* App details page: I agree that applying the maximum width would be good.
* List pages: this is already implemented and if you want to change the maximum width value you can experience it easily editing the code and building your local version from jhbuild or changing it live in the running process with GTK Inspector. You can even push the change yourself if you decide what value it should be.

The only concern is the overview page. I also like how it expands on wide screens and I think there is already some value where it stops expanding (you said 1314px, probably that's the value), probably it could be a little smaller but not so small to lose its nice appearance on large screens. I think we could apply some limit and see how it looks.

Whatever change we apply please keep in mind that the application should also handle correctly both much wider and much smaller screens, including the screens in vertical orientation.
Comment 4 Allan Day 2016-02-10 12:14:20 UTC
OK, so it sounds like the way forward is to try a larger maximum width for the lists and the app details pages, and a smaller maximum width for the overview page. If we don't manage to meet in the middle, that's OK: I'd be OK with having a different width for the overview page.

One concern I have though: what about the screenshots on the app details pages? I understand that the screenshots are generally only 752px wide: if we expand the width of the details pages, the screenshots won't be big enough to span the width.
Comment 5 Rafal Luzynski 2016-02-10 19:27:09 UTC
(In reply to Allan Day from comment #4)
> [...] I understand that the screenshots are generally only 752px wide: if
> we expand the width of the details pages, the screenshots won't be big
> enough to span the width.

That's true, so the screenshots box must be centered or start aligned. I hope it is OK. Another option is to scale the main screenshot but it would cause some loss of image quality. I think we should avoid this.
Comment 6 Rafal Luzynski 2016-02-11 00:26:59 UTC
Created attachment 320838 [details] [review]
Applies max-width=860 to the app details page

This patch looks huge but the only relevant changes are:

* put almost everything inside the GsFixedSizeBin with preferred-width=860;
* set the box_details halign=fill to let it actually fill the new maximum width;
* decrease its margins from 64 to 24 pixels to match the app lists (Installed, Search, etc.)

Other changes are just formatting, I recommend viewing it in space-ignoring mode.

Since the patch is actually huge and other developers are also working on the same file I'd like to get your review quickly. If you like this patch please commit it ASAP, if the only concern is the width value then please note that it can be changed easily in future. If you don't like it please just say it. Otherwise soon we may face conflicts and the patch will become unmergeable.
Comment 7 Allan Day 2016-02-11 09:50:59 UTC
Thanks for the patch, Rafal. I've experimented with different widths for the app pages and for the lists. To me it looks like the lists would be better even wider, somewhere in the 1000-1200px range. The app details pages could also be wider, although 1200 is a bit too wide: around 1000px seems OK there.

As expected, the screenshots are an issue, since they don't span the full width of the app pages.
Comment 8 Rafal Luzynski 2016-02-11 13:29:41 UTC
Tanks, Allan. Do you think we should ask Richard to push it to master now and only change the value 860 later? I'm asking this because I'm afraid of the git conflicts. If you think that applying a max width to the app details is a good idea but only not yet sure what the value should be then IMHO it's better to push it.

The app details width 860 pixels minus margins actually does not look much different than the current 752. The screebshots box is start (left) aligned, it's OK for now because all texts all also start aligned and seldom actually reach the right end. Centered would look too much shifted right. Of course this may be different if we change the width to 1000px or more.
Comment 9 Richard Hughes 2016-02-22 16:23:35 UTC
I think I'd rather merge such an invasive patch after we branch.
Comment 10 Rafal Luzynski 2016-04-01 10:33:21 UTC
Created attachment 325140 [details] [review]
Applies max-width=860 to the app details page (V2)

Second attempt - rebased, appliable.
Comment 11 Richard Hughes 2016-04-01 10:45:27 UTC
Created attachment 325142 [details]
what i see

Looks good in code, but looking at the screenshot I see the stars no longer align with the right side of the thumbnails -- is that intentional?
Comment 12 Rafal Luzynski 2016-04-02 20:57:44 UTC
OK, I think I know what you mean. Indeed, I have not noticed this. This is a consequence of applying a fixed width to this page. The current fixed width is 752px, this is also an actual width of the screenshots box. TL;DR answer is: if you don't want this appearance to be changed then let's use the "preferred-width"=800.

Here is a longer explanation. What this patch does is:

* use the GsFixedSizeBin to control the fixed width (in fact this is the maximum width so in future it will allow to collapse the window and fit in small screens, the current version uses the minimum width),
* increase the width value to 860px.

If you change the "preferred-width" value of GsFixedSizeBin to 800px, with 24px margins it will result with the content being 752px wide and make it look as it looks now. Note that in comment 7 Allan was thinking about 1000px. If you apply a larger width it will make the box_details_header expand and move the stars to the right, also it will expand the app description and the details at the bottom. The screenshots box will not expand.

You can use GtkInspector to play with different values of "preferred-width" property of gs_fixed_bin, also with "halign" property of box_details_screenshot.

If you want the stars and probably whole box_details_header to be aligned with the screenshots box we can use a GtkSizeGroup. But then please answer: what should happen if an app does not have screenshots or if it a component (e.g. an input method) which will never have screenshots?

Allan, what's your opinion?
Comment 13 Allan Day 2016-04-06 13:34:07 UTC
Instead of left-aligning the screenshots, why not expand the width of the main screenshot background? I think this would look better, since it would reinforce the left and right margin of the page content.
Comment 14 Rafal Luzynski 2016-04-08 21:31:04 UTC
Created attachment 325611 [details]
Example of the expanded main screenshot widget

Is that what you mean, Allan? Please note that my next step will be placing the thumbnails horizontally under the main screenshot on very small screens. Soon we will face the problem "ok, we have too little space to put the thumbnails at the right side of the main screenshot so fine to put them below but now we have too much horizontal space for just the screenshot." This would be the solution.

Other options I think of are:

- center the screenshots box;
- add two vertical lines marking the margins;
- add two horizontal lines above and below the screenshots marking the full width of the central area.
Comment 15 Allan Day 2016-04-11 09:58:20 UTC
(In reply to Rafal Luzynski from comment #14)
> Created attachment 325611 [details]
> Example of the expanded main screenshot widget
> 
> Is that what you mean, Allan?

Yes, that's right.

> Please note that my next step will be placing
> the thumbnails horizontally under the main screenshot on very small screens.

Sounds good to me!
Comment 16 Jakub Steiner 2016-04-11 10:09:30 UTC
I like the screenshot/thumbnail arrangement discussion. I would add to span the whole width of the block in case there is only one screenshot and no switcher is necessary (rather than keeping the space for mini thumbnails allocated as it is in the table version right now).
Comment 17 Rafal Luzynski 2016-04-11 10:27:24 UTC
Thanks, Allan, I think we will finish this patch soon.

(In reply to Jakub Steiner from comment #16)
> [...] I would add to span
> the whole width of the block in case there is only one screenshot and no
> switcher is necessary (rather than keeping the space for mini thumbnails
> allocated as it is in the table version right now).

I think this is how it works now and I'm (we are) not going to change it.
Comment 18 Jakub Steiner 2016-04-11 10:48:39 UTC
My bad, the current 3.20 version actually spans full width for single screenshots. We're good :)

https://raw.githubusercontent.com/gnome-design-team/gnome-mockups-software/master/wireframes/app-page-grid.png
Comment 19 Rafal Luzynski 2016-04-18 21:01:39 UTC
Created attachment 326295 [details] [review]
Expand the width of the main screenshot background

I'm sorry for being absent for so long.

Here is the second patch which does exactly what Allan asked for in a comment 13. Please apply it after applying the first patch.

Please note that these patches set the preferred width of the Details page to 860px. Feel free to set it to a different value if you prefer.
Comment 20 Rafal Luzynski 2016-04-25 08:47:02 UTC
Created attachment 326666 [details] [review]
Applies max-width=860 to the app details page (V3)

Just rebased to reflect the recent changes.
Comment 21 Richard Hughes 2016-04-25 09:28:12 UTC
Review of attachment 326295 [details] [review]:

Looks good, thanks.
Comment 22 Richard Hughes 2016-04-25 09:28:19 UTC
Review of attachment 326666 [details] [review]:

Looks good, thanks.
Comment 23 Rafal Luzynski 2016-05-23 23:19:01 UTC
Thank you, Richard, for pushing and I'm sorry for responding late. Now we have the following views (children of stack_main) with the following state:

* Installed,
* Updates,
* Extras,
* Search - max width of these four set to 860px in the fix for bug 758662.
* Moderate - newer view but copied from the above so also has max width = 860px
* Details - this fix sets the max width of the outer box to 860px, the inner box to 812px, the difference comes from the margins 2x24px
* Category - max width of the right panel is 1338px and should not be much smaller (fixed as bug 758669); left panel - minimum (!) width 220px and one can expect it won't be much wider.
* Loading - does not matter, this view is 480px wide and probably won't be wider.
* Overview - no maximum width set explicitly but the children force it to be 1338px, actually 1314px due to the margins 2x12px. It should not be smaller because this is also the minimum width to display 8 recommended application tiles.

Jakub, Allan, is there anything we can do here? If you want different max width values for these pages you can set them now. For other pages I suggest doing nothing. Can we close this bug report?
Comment 24 Richard Hughes 2017-02-17 11:53:48 UTC
(In reply to Rafal Luzynski from comment #23)
> Can we close this bug report?

I think so. Thanks all!