-
Notifications
You must be signed in to change notification settings - Fork 59
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
Adjustments according to new no data page layout #691
Conversation
bb2e559
to
d71e232
Compare
6542c95
to
0d4b061
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snake14 I'm aware that the align in that list isn't as beautiful as in the others. The problem is (as mentioned in the PR description) that the structure of the TagManager components results in a html structure that does not look like
but contains nested levels like
That makes it impossible to use the global layout as it requires a simple structure to render correctly. Btw. the list layout as it was use before didn't work correctly at all. It tried to intend all following lines using a padding or margin. That only works as long as the text fits in one line. Which already wasn't the case in a lot translations. |
@sgiehl I don't think there are any plans to refactor this page as it was recently redesigned. Part of the problem is that the components are used in multiple places, plus using components means that there are some unexpected divs inside of lists and we had to account for that. As far as the note container I mentioned, we simply indented that container (div or p) and let all the others follow their default wrapping behaviour. |
Refactor to remove tech debt/improve the markup shouldn't be based on how recently the page has been redesigned. Even components that have extra divs should be able to be put in lists and respect their styling, e.g. by having the div inside and not outside of the div and so on. Or use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to try to indent the note so that it matches the GA and GTM tabs, but it sounds like more work is needed in this tab, so we're probably fine merging this PR as it is.
I'm merging this one for now. We can add further improvements later |
Description:
The structure of the Tracking Code components is currently very complex. Tried to simplify that a bit, but it's still not possible to use the components to receive a clean html list structure. There are always
div
elements that aren't part of ali
, orli
elements that are surrounded bydiv
. The list for tag manager currently doesn't use the new global list style for decimal lists, as it only works with a clean html list structure. I guess for that the components would needed to be fully refactored...refs matomo-org/matomo#21247
Review