Live visitor log panel change padding for search referrer #20293
Conversation
Padding should be on the bottom not on top.
|
I realize this is only an issue when there are multiple single page visits after each other. Because when multiple pages are visited, that already makes the row high enough. But with single pages, the search referrer (in this case all Google) looks to be part of the next row: |
There was a problem hiding this comment.
Thanks for this @JackySw, it definitely improves readability 👍
- 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
- n/a Wording review done
- Code review done
- n/a Tests were added if useful/possible
- None Reviewed for breaking changes
- n/a Developer changelog updated if needed
- n/a Documentation added if needed
- n/a Existing documentation updated if needed
|
I'll revert this one. It actually causes regression in some places where the padding is expected. @bx80 Didn't you have a look at the UI test results before merging. There are actually a lot regressions/changes there: https://builds-artifacts.matomo.org/matomo-org/matomo/4.x-dev/4073665712/ @JackySw Feel free to recreate the PR. I guess instead of changing the existing CSS, we might need to add another more specific selector, which only changes it in the widget where it looks incorrect. If you can't find a proper selector feel free to open a bug report instead, so we will have a look at it at some point. |
|
Ok I understand, maybe it was a too easy fix. I will dive in deeper to understand the relation to other parts of these rows. I still think it has to change, because it's not correct that the search referrer is visually part of the row/visit below it. |
|
@sgiehl Sorry about that, I definitely did check the UI test results and there were no failed UI tests shown (which in hindsight should have been suspicious). Could there be some caching / latency issue with the build artifacts server? |
|
@bx80 Looks like for some reason the screenshots for the commits in the PR where not uploaded at all. See https://github.com/matomo-org/matomo/actions/runs/4055589262/jobs/6990587437#step:3:1517 |
The padding should be on the bottom not on top. The search engine referrer looks that way if it is part of the next row. By adding the padding on the bottom instead on top, it is in line with the other properties of that visit, then a white space and then the next row as it should.