-
Notifications
You must be signed in to change notification settings - Fork 327
Have a check for update button #8189
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
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? |
ee1b3ab
to
bce07e2
Compare
@jandubois - how does it look? |
Gentle reminder @jandubois , @mook-as ^^ |
As I've shown in #7516 (comment) the missing "Restart Now" button seems a separate issue from performing the update check right now. If the update button is missing (see the issue how to get into that state), then pressing the "Check Now" button will not make it re-appear; the button has no effect at all and the screen looks like this: |
I think this looks fine, but maybe we can include the same kind of "Last checked: 5 minutes ago" info below the "No updates available" line? Since this is not a button text, I would only capitalize the "No" but not the remaining words. |
Changes - add new button - add new event Signed-off-by: Vikalp Rusia <53312141+VikalpRusia@users.noreply.github.com>
bce07e2
to
0b91c64
Compare
Change
Details
How
update-check-now
to parentNote -> similar structure is there for apply-update
Screenshots
close #7516