-
Notifications
You must be signed in to change notification settings - Fork 309
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
Have a check for update button #8189
base: main
Are you sure you want to change the base?
Conversation
c49a403
to
168b01a
Compare
95ab78c
to
3c67250
Compare
4389bd5
to
e153aa6
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.
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.
Please squash your commits; we don't really need to add and remove the excess logging in the commit history. (We use merge commits instead of just squashing, because we often have useful intermediate commits.) It would also be useful to have more detailed commit messages (though in this case that is probably unnecessary).
8b7d6a8
to
ee1b3ab
Compare
Please re-trigger the pipeline as error was in the setup phase. |
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.
The code looks reasonable. @jandubois please check the UI and merge if you're fine with it.
align-items: center; | ||
} | ||
|
||
.btn-xs { |
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.
That looked too generic, but that's what all the other buttons with limited height do, so I guess that's okay‽
The button looks fine now, but when I click it, nothing happens. It feels like the button doesn't work. That is of course because there are no updates available. So I think we either need to add a section where we normally display the release notes, that shows the time we have checked, and there are no available updates. Alternatively we could display a popup dialog that provides the same info, something like this: But it feels like adding the info to the page itself (and not require the user to press a button in the dialog) is preferable. We have a similar issue on the Diagnostics page. But there we have a text below the button that will change to "Last run: a few seconds ago" when we click the button: But that mechanism would look add on the General page, which is not primarily about checking for updates. I have not tested this, but I wonder what happens if an update is actually available and I press the install button. I assume the update is downloaded, and the app will need to be restarted to install it. But will this happen even when the "automatically install updates" checkbox is not checked? |
Yup popup seems good will work on it |
I would prefer to display the text "No updates available" in the space where we would normally show that an update is available, with the release notes. That way the user doesn't have to click another button to dismiss the popup. This is also more in line with how Other buttons and errors/feedback work in Rancher Desktop (e.g. the "Rerun" button in diagnostics just updates the "last run" field. |
Can we display time also when was it last checked? |
adf8694
to
ee1b3ab
Compare
Changes - add new button - add new event Signed-off-by: Vikalp Rusia <[email protected]>
ee1b3ab
to
bce07e2
Compare
@jandubois - how does it look? |
Gentle reminder @jandubois , @mook-as ^^ |
Change
Details
How
update-check-now
to parentNote -> similar structure is there for apply-update
Screenshots
close #7516