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.

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

feat: improve Podman-Desktop update alert #6068

Merged
merged 8 commits into from Feb 29, 2024

Conversation

axel7083
Copy link
Contributor

@axel7083 axel7083 commented Feb 19, 2024

What does this PR do?

According to #5754.

When an update is available

  • If user is just starting up Podman Desktop from fresh, have a pop up dialog
  • if an update occurs a popup is spawned

A new preference section called Update has a property reminer with two value startup or never.

Screenshot / video of UI

starting podman-desktop

image

Clicking on status bar item when update available (status bar item)

image

What issues does this PR fix or reference?

Fixes #5754

How to test this PR?

  • Tests are covering the bug fix or the new feature

Testing manually

(1) change the version to 1.6.5 in the package.json at the root
(2) run yarn compile:current
(3) go to dist folder and start the portable executable

(Thx to @benoitf for this method)

@axel7083 axel7083 requested review from benoitf and a team as code owners February 19, 2024 13:48
@axel7083 axel7083 requested review from jeffmaury and cdrage and removed request for a team February 19, 2024 13:48
@axel7083 axel7083 changed the title feat: improve update logic feat: improve Podman-Desktop update alert Feb 19, 2024
@axel7083 axel7083 linked an issue Feb 20, 2024 that may be closed by this pull request
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

there is a refactoring and a new addition in the same PR so it's hard to track what is new, what is refactoring

I think having refactored first (extract to a separate class first) and then adding feature would have helped the review

@axel7083 axel7083 marked this pull request as draft February 27, 2024 10:17
@axel7083
Copy link
Contributor Author

there is a refactoring and a new addition in the same PR so it's hard to track what is new, what is refactoring

I think having refactored first (extract to a separate class first) and then adding feature would have helped the review

I made a didicated PR for the refactor in #6159 - keeping this in draft for now

@axel7083 axel7083 force-pushed the feature/improve-update-alert branch 2 times, most recently from 2e18e23 to e0e2d3b Compare February 28, 2024 13:21
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
@axel7083 axel7083 marked this pull request as ready for review February 29, 2024 10:13
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

when starting Podman Desktop I have the message

image

while if I click on the version icon I have

image

I like the fact to have the version number of the new version to be displayed
could we have the same sentence ?

about the buttons, I think I see too many 'Update' choice
I'm not sure about the 'Update Later' and 'Update never' (I think the never is looking odd as I can still click on the version to update)

should it be around 'Skip' for 'Update Later' and 'notifications' (or any other wording)

About the notify later, should it still prompt on every startup or should it be in case of a new version is released.

packages/main/src/plugin/updater.ts Outdated Show resolved Hide resolved
packages/main/src/plugin/updater.ts Outdated Show resolved Hide resolved
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
@axel7083
Copy link
Contributor Author

@benoitf I took some liberty with the original mockup #5754

starting podman-desktop

image

Clicking on v1.6.5 (status bar item)

image

Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

I find it way better now 🎉

it's not related to this PR but maybe we should change our wording when there is a new version as the last version that we see at the end of the sentence is the current version

so reading it quickly I have the feeling that it's asking to update to the version number that we have

If others share the same concern, we could do follow-up, if I'm the only one then we can just ignore :-)

@benoitf
Copy link
Collaborator

benoitf commented Feb 29, 2024

before merging, you'll need to update the body of the PR with accurate screenshot/video

@axel7083 axel7083 merged commit 78d3b1f into containers:main Feb 29, 2024
8 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.8.0 milestone Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvements to new version alerts UX: Improvements to new version alerts
3 participants