@flamisz opened this Pull Request on March 16th 2021 Contributor

Description:

fixes #17162

Tested in FF, Chrome and IE11.
Tested with responsive design mode as well.

Review

  • [ ] Functional review done
  • [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@flamisz commented on March 16th 2021 Contributor

Couple of screenshots, to see how it looks like. The only issue is on mobile, when we have two calendars, you have to scroll horizontally. It was like this, I didn't touch that part, and probably that's all right. If it's all right, I can update the UI tests (probably will fail a couple).

Screen Shot 2021-03-16 at 5 26 39 PMScreen Shot 2021-03-16 at 5 26 46 PMScreen Shot 2021-03-16 at 5 26 56 PMScreen Shot 2021-03-16 at 5 27 18 PMScreen Shot 2021-03-16 at 5 27 23 PM
@sgiehl commented on March 16th 2021 Member

Wondering if it might look a bit better if the period options would be placed with a fixed distance to the calendar.

@flamisz commented on March 16th 2021 Contributor

Wondering if it might look a bit better if the period options would be placed with a fixed distance to the calendar.

Yeah, probably. Currently, that's a table, which is not the best practise these days. I will look into it what's the quickest solution.

@flamisz commented on March 17th 2021 Contributor

I changed the table layout to flex. It looks like this now:

Screen Shot 2021-03-17 at 4 31 00 PM

This is a nicer solution.

Tested on IE11.

@sgiehl commented on March 23rd 2021 Member

The code itself looks fine so far. The UI tests needs to be updated, but not sure regarding the UI overall. Maybe @tsteur or @mattab want to have a look

@tsteur commented on March 23rd 2021 Member

I'd actually prefer the original look and also how it was shown in Firefox be totally fine for me if the compare is over two lines (in some ways you could argue it's even better over two lines if there was a bit more gap on top of compare to)
image

Maybe the width of the calendar could be made only slightly wider and if it breaks in some languages we wouldn't mind because it would still kind of work.

Seems @mattab is not happy with the two lines though so be good to comment @mattab

@mattab commented on March 25th 2021 Member

Would it be possible to make it look like this?

ideal look - Screenshot from 2021-03-25 14-14-01

so same as https://github.com/matomo-org/matomo/pull/17348#issuecomment-800767507 but:

  • the calendar panel is less wide
  • the select list less wide
  • so everything fits neatly to the left
@tsteur commented on March 25th 2021 Member

@mattab problem are also eg translations etc.

from a UX point of view the way you want it is btw some confusing because you look from top to bottom and you see various periods listed on the right and you look further down where it then suddenly says previous periods and it's not clear what/how that relates to. I would say to show the previous period select only when the checkbox is activated. It might cause some jumping but at least the majority of the use cases would be simplified and eg GA does the same etc (because this select is confusing where you want it placed).
image

@mattab commented on March 25th 2021 Member

Agreed, that would be a great improvement to show the ... to [ previous period ] after a click on [ ] Compare

@flamisz commented on March 26th 2021 Contributor

@mattab and @tsteur to show only the [previous period] after ticking the compare sounds good to me.
My question is, do we want to change the layout in any way? Like make it always on the second line, or make it slightly wider or just keep it as it is, so someone got it in one line, someone 2 lines?

@mattab commented on March 26th 2021 Member

My question is, do we want to change the layout in any way

Ideally looks like this (imagine the checkbox should be clicked here, and the white space on the right should be the same width as on the left)

ideal look - Screenshot from 2021-03-25 14-14-01

@flamisz commented on March 26th 2021 Contributor

Ideally looks like this (imagine the checkbox should be clicked here, and the white space on the right should be the same width as on the left)

Yeah, it will work, but only in English. So my next question is what about other languages? Like in German, the word is longer, so won't fit. I can make the fonts smaller so even longer words can fit, only the very long ones will break into 2 lines.
Or if the word is longer, we'd like to grow the box?
ping @mattab

@mattab commented on March 29th 2021 Member

Or if the word is longer, we'd like to grow the box?

for visual consistency it's better to preserve the box width, so instead you can do the other solution " long ones will break into 2 lines." :+1:

@flamisz commented on March 29th 2021 Contributor

I tweaked the css a little bit, works well with even German translation, tested it in IE11/Chrome/Firefox.

https://user-images.githubusercontent.com/6749509/112792587-4030e980-90c0-11eb-9b77-79d7d3fefe8b.mov

@flamisz commented on April 6th 2021 Contributor

👋 @mattab can you please check the latest version of this? There is a short video in my previous comment. Thanks

@mattab commented on April 7th 2021 Member

Looks good to me @flamisz :+1: Nice improvement

This Pull Request was closed on April 7th 2021
Powered by GitHub Issue Mirror