@mnapoli opened this Pull Request on June 11th 2015 Member

Changed slightly the position of icons in the visitor log so that it's visually a little better and it will not cause screenshot tests to randomly fail (at least that's what I hope…).

Basically what I've done is remove the float left/float right for the 2 groups of icons and instead display them on top of one another.

You can see the diff here: http://builds-artifacts.piwik.org/ui-tests.fix-ui-tests-visitor-log/13240.7/screenshot-diffs/diffviewer.html


capture d ecran 2015-06-11 a 14 03 18


capture d ecran 2015-06-11 a 14 03 14

@mattab commented on June 12th 2015 Owner

I tried the branch and it does not look quite good to have two rows of icon above each other I think

One way to make the visitor log look nice is to redesign it, in #7909. Maybe we could use the card design like was done in #8002 and change the layout a bit. (maybe the visitor log could look bit more consistent with Visitor Profile?)

If we want to solve the random test UI while waiting for #7909 maybe you could put the icon Flag+Returning visitor in the left column, below all other user metadata, left aligned with same spacing as other text lines. (quickly tried it and it looked good imho)

@mnapoli commented on June 12th 2015 Member

OK will try that!

@mnapoli commented on June 15th 2015 Member

Updated to this:

capture d ecran 2015-06-15 a 09 51 33

Is that what you meant @mattab?

@mattab commented on June 16th 2015 Owner

Yes it's better this way.. Feedback: we need to have the same space on top and bottom of the icons. Got: less margin on top than on bottom. Expected: more space on top of icons?

PS: do you think it will solve the random UI test failure? IIRC the UI test fails because of the "Goal conversion / ecommerce" block positioning and this block hasn't moved. But maybe i'm wrong and just moving the flag + returning icons, will fix it :+1:

@mnapoli commented on June 16th 2015 Member

It's the same space as it is today, it's like "paragraphs"…

@mattab commented on June 16th 2015 Owner

In general in Piwik there are many issues with spacing, so it's nice to leave things better than when we found. I pointed this one out because it would be nice to slowly fix those inconsistent spacing issues everywhere. Tomorrow with @tsteur we'll take a look at spacing in the UI especially around titles. Feel free to skip this if you don't care, as it's not a new issue

@mnapoli commented on June 16th 2015 Member
  1. I don't think removing the space will make things look better (right now things are grouped into whatever way, but this is a group not a random extra new line)
  2. to make this look actually better, it requires a lot of work that has nothing to do with this change that is mostly about fixing our UI tests
  3. every back and forth on this topic takes time because we have poor development environments and tools

I just want to try and fix tests because it's a huge pain every single day. I want to move forward, those back and forth only kill the motivation to tackle those long-standing side-issues.

@mattab commented on June 16th 2015 Owner

Sure I got carried away, let's get moving and stay focused on your goal here which is fixing a UI test :+1:

This Pull Request was closed on June 16th 2015
Powered by GitHub Issue Mirror