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
revert refresh button #19522
revert refresh button #19522
Conversation
I would personally at least keep the keyboard shortcut. But that might be not a decision I'm allowed to make. |
Keyboard shortcut be good to have indeed |
@justinvelluppillai should this one go into 4.11 ? Otherwise the button would be added with 4.11. and removed again with 4.12. Which imho wouldn't be good. |
@sgiehl yes that is the intent here. @peterhashair this would need to be rebased of the |
revert refresh bottom
add refresh short cut
Co-authored-by: Stefan Giehl <stefan@matomo.org>
@peterhashair I have rebased the branch. Might be possible some of the screenshots might need another update due to the change.
Or what might be an even better handling: Give feedback on each comment like "I've pushed a fix", answer questions or ask a question if it wasn't clear. Afterwards the reviewer can resolve the comments himself in the next review round. |
update screenshots and add translation
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 have looked through all the screenshots you have already updated. It seems the changes on plugins/Transitions/tests/UI/expected-screenshots/Transitions_transitions_popup_urls.png
are not caused by changes of this PR, so you should avoid updating it.
Also tests/UI/expected-screenshots/UIIntegrationTest_shortcuts.png
seems not be updated to the latest version, as it still lacks the refresh shortcut.
I've now seen that a couple of times on your PRs that screenshots are updated, where they shouldn't have been. In general you should avoid simply using the sync command to download all screenshots and afterwards add all changes and push them. It is mandatory to look through all changes on the artifacts server before and validate that the changes on a screenshot are caused by your changes. Even if changes in a screenshot look valid, avoid updating it in a PR that didn't cause the changes.
Co-authored-by: Stefan Giehl <stefan@matomo.org>
This reverts commit b142042.
update shortcut screenshot and revert Transition screenshots
@sgiehl will pay more attention to the screenshots updates from now on 👍. Updated it. |
Description:
Fixes: #19515
revert refresh button
Review