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
Improve alignment in Visits > real-time by hiding parent of period selector #19820
Conversation
hide parent div of period selector
real-time
to improve alienment.
real-time
to improve alienment. real-time
to improve alignment
@@ -250,7 +250,7 @@ export default defineComponent({ | |||
}, | |||
mounted() { | |||
Matomo.on('hidePeriodSelector', () => { | |||
window.$(this.$refs.root as HTMLElement).hide(); | |||
window.$(this.$refs.root as HTMLElement).parent().hide(); |
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.
maybe hide the parent only if it has [vue-entry]? this way if PeriodSelector is used directly in a Vue template, an element in the parent component wont be accidentally hidden
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.
@diosmosis ah I found the issue case, the parent doesn't have [vue-entry], but should hide. maybe [vue-entry] doesn't work.
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.
👍
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.
Doesn't the parent div always have the id periodString
in case it should be hidden?
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.
@sgiehl we probably can check if the id has periodString
good point.
update hide only on vue-entry
revert vue-entry
update tests and add Id for the selector
update screenshot
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
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.
Tests well, segment selector is properly aligned on the real time visits view and the period selector is re-enabled on other screens without issue.
- 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
- Tests updated 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
real-time
to improve alignment
Description:
Fixes:#19818
hide parent div of period selector in Visits->
real-time
to improve alignment.Review