Patch #30435

Change #main block to a flexbox + additional minor fixes to it's direct children.

Added by Max Johansson about 1 month ago. Updated 7 days ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:UI
Target version:4.1.0

Description

What this patch does:
1. It moves the sidebar to be rendered after the content block in base.html.erb, since natural flow of HTML should be from left to right, so no point in rendering sidebar before the content block, because it doesn't match our base layout.

2. It sets the #main block to a flexbox and removes a redundant background color definition rule from it.

3. It changes the % based width of the sidebar to a fixed 220px (if you think it should be wider, feedback is welcome), because on wide screens 22% is quite a lot of space!

4. #content block is the only one that will stretch now.

main-block-fixes.diff Magnifier (2.46 KB) Max Johansson, 2019-01-15 12:41

main-block-fixes-no-swap.diff Magnifier (1.57 KB) Max Johansson, 2019-01-17 07:45

sidebar.png (289 KB) Marius BALTEANU, 2019-01-17 21:28

main-block-fixes-no-swap-min-max.diff Magnifier (1.67 KB) Max Johansson, 2019-02-09 00:34

main-block-fixes-no-swap-min-max_no_shrink.diff Magnifier - Max J.'s patch with flexbox constraints (1.68 KB) Bernhard Rohloff, 2019-02-14 18:28

History

#1 Updated by Marius BALTEANU about 1 month ago

  • Subject changed from Change #main block to a flexbox + additional minor fixer to it's direct children. to Change #main block to a flexbox + additional minor fixes to it's direct children.

#2 Updated by Go MAEDA about 1 month ago

  • Target version set to 4.1.0

It looks nice. Setting the target version to 4.1.0.

#3 Updated by Marius BALTEANU about 1 month ago

Please do not commit because I want to test more these changes. I looked yesterday for few seconds and I saw some problems.

#4 Updated by Marius BALTEANU about 1 month ago

  • Assignee set to Marius BALTEANU

#5 Updated by Bernhard Rohloff about 1 month ago

The patch looks good to me, although I'm not sure if the width is sufficient for the text length of everbody's saved queries. It also wraps the heading of "Custom queries" if you choose German ( it's "Benutzerdefinierte Abfragen" ) as interface language. No big deal to me but I think it's worth mentioning.

Overall, I think the patch is a great improvement.

+1

#6 Updated by Max Johansson about 1 month ago

I'm also wondering about the nature of this rule, does anybody have idea what it should do?

* html #content{ width: 75%; padding-left: 0; margin-top: 0px; padding: 6px 10px 10px 10px;}

because in the default version, just one line above it would already be defined as:

#content{ width: 75%; padding-left: 0; margin-top: 0px; padding: 6px 10px 10px 10px;}

I have a feeling, Internet Explorer is guilty :)

#7 Updated by Marius BALTEANU about 1 month ago

In my environment, the #content loads first with 100% width and right after, the sidebar which makes the screen "shake".

#8 Updated by Max Johansson about 1 month ago

Marius BALTEANU wrote:

In my environment, the #content loads first with 100% width and right after, the sidebar which makes the screen "shake".

Marius, I looked into it, so I'm attaching alternative patch without element swapping in base.html.erb to see if this one will not spawn shaking, the previous patch must be reverted before applying this one.

The alternative method could as well be to set #main a flow-direction: row; instead of row-reverse and than add order: 1; for the #content and order: 2; for the #sidebar, but I guess this isn't needed now, because we only have 2 containers, but it could be useful if we re to spawn an additional tile under the sidebar in the future for instance, else in the favor of less rules, it is imho just be simpler to reverse it. :)

#9 Updated by Marius BALTEANU about 1 month ago

Thanks Max for updating the patch, it works well with row-reverse.

I agree with Bernhard, the sidebar width (220px) is too small even for small screens, please see the below screenshot from my 13 inch screen:

1. width: 220px;

What do you think if we keep the existing width: 22% rule and we add a max-width of 280px or 300px?

#10 Updated by Max Johansson about 1 month ago

I think width: 22% will anyway often exceed it's maximum limitation of max-width, maybe we could try min-width: 220px and max-width: 280px or 300px?
In theory sidebar's content should then expand the sidebar in width within these margins a bit depending on what it contains. :)

#11 Updated by Max Johansson about 1 month ago

There is a gridbox design suggestion now pending for further discussion, so this ticket will be affected if we will decide to move #main-menu inside #main container, ultimately making this patch obsolete, because it will then become a 3 tiled gridbox instead of a flexbox. Please see #30451#note-6 for reference.

#12 Updated by Marius BALTEANU about 1 month ago

  • Assignee deleted (Marius BALTEANU)

#13 Updated by Max Johansson 13 days ago

I think it will be better to ignore #30451 for now, so attaching the patch with min-width and max-width rules for the sidebar, the content of the sidebar will now expand it up to 340px if contains long elements, if there is no need, it will reduce up to 220px. I think it's the most suitable resolution for everyone.
I also removed some redundant rules this time, like:

* html #sidebar{ width: 22%; }
* html #content{ width: 75%; padding-left: 0; margin-top: 0px; padding: 6px 10px 10px 10px;}

#14 Updated by Go MAEDA 8 days ago

Max Johansson wrote:

I think it will be better to ignore #30451 for now, so attaching the patch with min-width and max-width rules for the sidebar, the content of the sidebar will now expand it up to 340px if contains long elements, if there is no need, it will reduce up to 220px.

Thank you for the patch. Narrower sidebar than the current implementation should be nice for many people.

I tried the patch but I found that the sidebar hardly expands even if there is a custom query with a long name and the width of the browser window is around 1800px. With my Firefox, the width of the sidebar is 261px at 1024px window and 271px at 1800px window.

How can I see a sidebar with a width of 340px?

#15 Updated by Bernhard Rohloff 7 days ago

Go MAEDA wrote:

I tried the patch but I found that the sidebar hardly expands even if there is a custom query with a long name and the width of the browser window is around 1800px. With my Firefox, the width of the sidebar is 261px at 1024px window and 271px at 1800px window.

How can I see a sidebar with a width of 340px?

I can confirm the behavior you described. I think is an issue with the flexbox constraints.
Here is a modified version of Max's patch which prevents the sidebar from shrinking. Can you check if it's working now?

Also available in: Atom PDF