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.

Sign up for GitHub

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

Hide update/version info when auto-update disabled #21384

Merged
merged 2 commits into from
Oct 16, 2023
Merged

Hide update/version info when auto-update disabled #21384

merged 2 commits into from
Oct 16, 2023

Conversation

michalkleiner
Copy link
Contributor

@michalkleiner michalkleiner commented Oct 9, 2023 •
edited
Loading

Description:

Ref. DEV-17254.

Disabling the update info widget completely when auto updates are disabled.

Review

@michalkleiner michalkleiner added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. c: Design / UI For issues that impact Matomo's user interface or the design overall. Needs Review PRs that need a code review labels Oct 9, 2023
@michalkleiner michalkleiner requested a review from a team October 9, 2023 10:55
@michalkleiner michalkleiner added this to the 5.0.0 milestone Oct 9, 2023
@sgiehl sgiehl removed the Needs Review PRs that need a code review label Oct 12, 2023
sgiehl
sgiehl previously requested changes Oct 12, 2023
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned before, this might not be the approach we want to go with. It might fix the current problem, but has the potential to cause other problems in the future.
There could e.g. be an element where the child element isn't displayed on purpose until some data was loaded or similar...

@michalkleiner michalkleiner force-pushed the dev-17254-top-controls-visibility branch from 265a37d to 3f13175 Compare October 16, 2023 06:00
@michalkleiner michalkleiner changed the title Extend display:none check to the child element in case it's hidden at that level Hide update/version info when auto-update disabled Oct 16, 2023
@michalkleiner
Copy link
Contributor Author

Updated approach as discussed to disable the whole update info widget when auto update is disabled.

@michalkleiner michalkleiner added the Needs Review PRs that need a code review label Oct 16, 2023
@michalkleiner michalkleiner requested a review from a team October 16, 2023 06:04
@michalkleiner
Let auto-update enabled control the whole logic
ccbcd23 
The isAutoUpdateEnabled flag actually means the instance can talk to Matomo servers at all, so there wouldn't be any checks
if it was false anyway.
@michalkleiner michalkleiner requested a review from a team October 16, 2023 08:59
@mneudert mneudert dismissed sgiehl’s stale review October 16, 2023 17:29

Changes addressed

@michalkleiner michalkleiner merged commit 1b2e4fa into 5.x-dev Oct 16, 2023
18 of 20 checks passed
@michalkleiner michalkleiner deleted the dev-17254-top-controls-visibility branch October 16, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mneudert mneudert mneudert approved these changes

@caddoo caddoo caddoo approved these changes

@sgiehl sgiehl sgiehl left review comments

Assignees
No one assigned
Labels
c: Design / UI For issues that impact Matomo's user interface or the design overall. Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@michalkleiner @mneudert @caddoo @sgiehl