@diosmosis opened this Pull Request on January 8th 2021 Member

Description:

Adds a new notification type (not made public or advertised) for category/subcategory documentation. Also changes the styling for the in-report help icon, but doesn't make it permanently visible. Might be a good idea to do that I guess, but I wasn't sure.

Fixes #13716

image

Review

  • [ ] Functional review done
  • [ ] 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
  • [x] 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
@diosmosis commented on January 14th 2021 Member

@mattab @tsteur @sgiehl this is ready for review (code + UI)

@diosmosis commented on January 18th 2021 Member

@mattab / @tsteur can you provide a UI review?

@tsteur commented on January 18th 2021 Member

@diosmosis currently, it shows the help icons for the currently selected ones
image

The help icon on the main category talks about the reporting page but probably needs to talk about the "visitors report pages"
image

I don't know if it's easily possible but be great to adjust the titles like below

  • How does the "Visitors" reporting section help me? // replace Visitors and visits log with the category/page name..
  • How does the "Visits Log" reporting page help me?

If it's too complicated can also not do it.

And generally I wonder if it was better to only show the help icons for the currently hovered reporting category/page. Because we then adjusted the text to not refer to the current reporting page this would be possible. Of course it might be confusing to see a report help for one page in the context of a different page so we could also additionally switch the page and show the help text at once. That's just a thought though because I found it visually bit cluttered when hovering another menu item and the other two question marks are still shown. Eg here I hover "real time map" and I wonder if the other two question marks should disappear and it should be instead only be shown for the real time map.
image

A problem be though that when clicking on the reporting category there is no page to show so maybe it doesn't work. For sections we could otherwise just show it as part of the current page though maybe. Personally, I would maybe show the help icon only for reporting pages.

Waiting for the thoughts from @mattab

@diosmosis commented on January 19th 2021 Member

If it's too complicated can also not do it.

It's do-able.

Of course it might be confusing to see a report help for one page in the context of a different page so we could also additionally switch the page and show the help text at once.

Switching the page is a possibility. I can try this and we can revert if it's weird. Though actually, seeing a help icon on hover for every category on the left might be weird too... I guess we can do it on hover of a single menu item. I'll try this.

Personally, I would maybe show the help icon only for reporting pages.

Makes sense for a first version.

@mattab commented on January 20th 2021 Member

The UI looks good overall :+1: (feedback below)

Personally, I would maybe show the help icon only for reporting pages.

Agreed :+1: the help icon can be removed from the top menus and only show for the current reporting page (on hover).

Also a small note but in terms of UX, because the icon is tiny (good for visual effect), it's (very) hard to click on it.
Could we make the clickable area bigger around icon, so it's easier to hover on it and click it? (if i had to guess, making clickable area 2x wider and 2x higher could work)

Also on hover on the icon, could we maybe show the colored icon? (to show some feedback and hint that clicking might do something). (similar to how it shows blue when hovering the icon next to a report's title)

@diosmosis commented on January 20th 2021 Member

@mattab / @tsteur applied all feedback

@sgiehl commented on January 20th 2021 Member

btw. should we at some point mention those category classes on https://developer.matomo.org/guides/menus ?

@tsteur commented on January 20th 2021 Member

:+1: now only need to wait for the help texts. We could already merge though and them later to avoid more merge conflicts later

Powered by GitHub Issue Mirror