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

Performance report table not displayed properly #17242

Closed
Findus23 opened this issue Feb 22, 2021 · 20 comments
Closed

Performance report table not displayed properly #17242

Findus23 opened this issue Feb 22, 2021 · 20 comments
Assignees
Labels
c: Design / UI For issues that impact Matomo's user interface or the design overall. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone

Comments

@Findus23
Copy link
Member

reported in https://forum.matomo.org/t/version-4-2-0-darstellungsfehler/40770

This might be a regression of 4.2.0.

The tables in the Behaviour -> Performance reports have a very wide first column when displayed in German (English seems to be fine) causing a horizontal scrollbar to be needed to see all columns.

grafik

@Findus23 Findus23 added c: Design / UI For issues that impact Matomo's user interface or the design overall. Regression Indicates a feature used to work in a certain way but it no longer does even though it should. labels Feb 22, 2021
@tsteur tsteur added this to the 4.2.0 milestone Feb 22, 2021
@tsteur
Copy link
Member

tsteur commented Feb 22, 2021

Be great to check if this is indeed a regression (not sure we changed anything there or not). If not, feel free to remove the regression label and unassign the milestone.

@mattab mattab modified the milestones: 4.2.0, 4.3.0 Feb 24, 2021
@Findus23
Copy link
Member Author

Findus23 commented Mar 1, 2021

On demo.matomo.cloud (which is still on 4.1.1) this is displayed perfectly fine, so I'm pretty sure this is a 4.2.0 regression

@flamisz flamisz self-assigned this Mar 4, 2021
@flamisz
Copy link
Contributor

flamisz commented Mar 4, 2021

The table width difference is coming from the header row: in German, the worlds are longer, like Avg. vs Durchschnittliche and we don't make them shorter, if they long we use the whole word.

There was a language file update in January which made it to version 4.2:
#17137

That causes the issue.

Any recommendation to solve it? Modify the logic of how we show the labels in the header row? Use some max-width? Or modifying the language file itself?

ping @tsteur @Findus23

@sgiehl
Copy link
Member

sgiehl commented Mar 4, 2021

Changing the translations won't make much sense, as it could be changed again anytime, also the words might be much longer in other languages.

@flamisz
Copy link
Contributor

flamisz commented Mar 4, 2021

Yes, that's true. I can modify the css of the header rows to cut the text if too long (hovering will show some extra info anyway).

@Findus23
Copy link
Member Author

Findus23 commented Mar 4, 2021

I'm not the biggest fan of truncating the header rows.
A German user would then see something like

Eindeuti...| Durchsch... | Durchsch...| Durchsch...| Durchsch...| Durchsch...| Durchsch...| Durchsch...

which doesn't really feel that much more useful than the current status.

To be fair, I can't think of a better solution. I don't even know a way to make the German translation any shorter.

@sgiehl
Copy link
Member

sgiehl commented Mar 4, 2021

Truncating doesn't make much sense though. If we do that it would be even better to cut out the middle, so you see beginning and end, which should make it easier to identify columns.
But maybe it would be better to limit the width of the columns and maybe reduce the fontsize using javascript until it fits (or reaches a certain minimum). might look a bit strange if the column headers have different font sizes... not sure, but don't have a better idea

@tsteur
Copy link
Member

tsteur commented Mar 4, 2021

We could also just leave things the way they are as it's maybe not too bad and smaller font size and other things may be bit more of a compromise?

@diosmosis
Copy link
Member

This suggestion might be a bit late, but if nothing else provides a good solution, we could break individual long words up and allow multi-line cell headers (if not allowed already). For example:

Visits | Averylong- | Actions | etc.
       | word       |         |

@flamisz
Copy link
Contributor

flamisz commented Mar 5, 2021

I like @diosmosis suggestion. That was my original idea, just wrap the text for equal lines, but looked weird, but just because I haven't added the - on the end 😄. Headers are already multi-line, we just break the lines only on spaces, that's why long words make wide columns.

I'll make some screenshots for different solutions and share it here.

@flamisz
Copy link
Contributor

flamisz commented Mar 5, 2021

The three variants:

Current solution:
Screen Shot 2021-03-05 at 4 08 31 PM
Solution with truncating words (twig filter):
Screen Shot 2021-03-05 at 4 07 51 PM

Solution with hyphens (this is css):
Screen Shot 2021-03-05 at 4 25 43 PM

The 3rd one is very simple, I can just create a .hyphenate css class and add that in the twig file.

What do you prefer? @diosmosis @tsteur @sgiehl @Findus23

@diosmosis
Copy link
Member

I think I'll save my opinion until german speakers weigh in because I have no idea if that's more readable or not. :)

@sgiehl
Copy link
Member

sgiehl commented Mar 5, 2021

For german the last option would work. But can't say anything about other languages.
@flamisz are you using something like the css hyphens setting?

@flamisz
Copy link
Contributor

flamisz commented Mar 5, 2021

@sgiehl I used this for the screenshot, but planning to double check how it works in different browsers:

.hyphenate {
  word-wrap: break-word;
  overflow-wrap: break-word;

  -webkit-hyphens: auto;
  -moz-hyphens: auto;
  hyphens: auto;
}

@flamisz
Copy link
Contributor

flamisz commented Mar 6, 2021

Unfortunately the css solution is not perfect. It works well in firefox and safari for me, but in chrome it looks like this:

Screen Shot 2021-03-07 at 9 41 35 AM

We can add this css, probably doesn't hurt anything, but won't be perfect for everyone, on every language, and on every browser.

The other option is to revisit how datatable calculates the width and what css we use there, but datatable is huge and we use it everywhere.

@tsteur what do you think? Keep it as it is? Use this css? Or rethink how we calculate the width?

@tsteur
Copy link
Member

tsteur commented Mar 7, 2021

Hi, just checked using Durchschn. instead of Durchschnittliche improves this significantly. There's still a bit of scrolling but that's OK. The label column has a normal/expected width then. Considering in English we're also using Avg. and for other columns in German we also use Durchschn. so it's not even really consistent. @sgiehl or @Findus23 could someone adjust the German translations for these columns? Could also check if some other columns need to be adjusted. This be a quick win and if then there are still some issues for another language where the appreviation is used then we could still look eventually further into it.

@sgiehl
Copy link
Member

sgiehl commented Mar 8, 2021

@tsteur Changed it to Durchschn. for all texts where the English version uses avg.

@tsteur
Copy link
Member

tsteur commented Mar 8, 2021

Great, thanks @sgiehl 👍 I think we can close this issue?

@sgiehl
Copy link
Member

sgiehl commented Mar 8, 2021

It might actually happen again for any column with a longer name in any language. But we can always reconsider fixing it another way I guess

@tsteur
Copy link
Member

tsteur commented Mar 8, 2021

Yes that's the idea. If there are issues again in other languages where they are using the abbreviation and the label column gets way too wide then we can look into it again if needed.

@tsteur tsteur closed this as completed Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Design / UI For issues that impact Matomo's user interface or the design overall. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants