Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure empty report tables also use max width #19819

Closed
tsteur opened this issue Oct 5, 2022 · 6 comments · Fixed by #19822
Closed

Ensure empty report tables also use max width #19819

tsteur opened this issue Oct 5, 2022 · 6 comments · Fixed by #19822
Assignees
@sgiehl
Labels
c: Design / UI For issues that impact Matomo's user interface or the design overall.
Milestone
4.12.1

Comments

@tsteur
Copy link
Member

tsteur commented Oct 5, 2022

Not sure if that was on purpose or a regression. Previously, they were limited to a certain width but now in 4.12 they utilise the full screen width.

For example it looked like this in 4.11

image

But now in 4.12 they use the full width.

I couldn't find anything in the changelog and assume this is a regression
@tsteur tsteur added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Oct 5, 2022
@peterhashair
Copy link
Contributor

peterhashair commented Oct 5, 2022 •
edited

@tsteur It's a maybe on purpose. the max width used to 1200px injected by javascript. But I believe this only applies when there is data inside the table, if there is no data, it shows as full width.

matomo/plugins/CoreHome/javascripts/dataTable.js

Line 418 in 64e8473

function setMaxTableWidthIfNeeded (domElem, maxTableWidth)

@peterhashair peterhashair added Regression Indicates a feature used to work in a certain way but it no longer does even though it should. c: Design / UI For issues that impact Matomo's user interface or the design overall. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. Regression Indicates a feature used to work in a certain way but it no longer does even though it should. labels Oct 5, 2022
@justinvelluppillai justinvelluppillai added this to the 4.12.1 milestone Oct 5, 2022
@sgiehl sgiehl self-assigned this Oct 6, 2022
@sgiehl
Copy link
Member

sgiehl commented Oct 6, 2022

I'll set up a PR to fix that for empty tables...

@tsteur would you mind to confirm, if that only happens for empty tables, or also for some tables with data?

@sgiehl sgiehl mentioned this issue Oct 6, 2022
Merged
11 tasks
@tsteur
Copy link
Member Author

tsteur commented Oct 6, 2022

@sgiehl it looks to me that this happens also for non-empty reports

@sgiehl
Copy link
Member

sgiehl commented Oct 7, 2022

@tsteur can you give some examples?

@tsteur
Copy link
Member Author

tsteur commented Oct 10, 2022

@sgiehl clicking through a few reports I actually couldn't find any examples. There were quite a few reports that had full width but turns out this was already the case with Matomo 4.11. Not sure if this regressed for other reports earlier some time or if it's always been that way. For example https://demo.matomo.cloud/index.php?module=CoreHome&action=index&idSite=1&period=day&date=yesterday#?idSite=1&period=day&date=yesterday&category=Funnels_Funnels&subcategory=2 . Likely it's been always that way on purpose and that's not a regression. So there should be no issue 👍

@sgiehl
Copy link
Member

sgiehl commented Oct 10, 2022

Ok. Then this should be fixed with the referenced PR.

@justinvelluppillai justinvelluppillai changed the title Reports have full width in Matomo 4.12 Ensure empty report tables also use max width Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sgiehl sgiehl

Labels
c: Design / UI For issues that impact Matomo's user interface or the design overall.
Projects
None yet
Milestone
4.12.1
Development

Successfully merging a pull request may close this issue.

Ensure empty tables also use max width matomo-org/matomo
4 participants
@tsteur @justinvelluppillai @sgiehl @peterhashair