fix: release notes banner update button by SoniaSandler · Pull Request #9371 · podman-desktop/podman-desktop (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation4 Commits6 Checks13 Files changed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Signed-off-by: Sonia Sandler ssandler@redhat.com
What does this PR do?
Fixes the release notes banner update button bug where the update button still shows up even after updating to the latest release.
Screenshot / video of UI
What issues does this PR fix or reference?
How to test this PR?
- Tests are covering the bug fix or the new feature
Signed-off-by: Sonia Sandler ssandler@redhat.com
Signed-off-by: Sonia Sandler ssandler@redhat.com
Signed-off-by: Sonia Sandler ssandler@redhat.com
Signed-off-by: Sonia Sandler ssandler@redhat.com
Comment on lines 19 to 44
| import type { Writable } from 'svelte/store'; |
|---|
| import { writable } from 'svelte/store'; |
| let podmanDesktopUpdateAvailable = false; |
| try { |
| podmanDesktopUpdateAvailable = await window.podmanDesktopUpdateAvailable(); |
| } catch (e) { |
| console.log('Cannot check for update'); |
| } |
| export const updateAvailable = setup(); |
| export function setup(): Writable { |
| const store = writable(podmanDesktopUpdateAvailable); |
| window.events?.receive('app-update-available', isUpdateAvailable => { |
| if (isUpdateAvailable) { |
| updateAvailable.set(true); |
| } else { |
| updateAvailable.set(false); |
| } |
| }); |
| return store; |
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we go with a store, it should be like the other one and use the new EventStore<....> pattern (where you can add events that you're interested in)
you might look at existing stores importing EventStore
Signed-off-by: Sonia Sandler ssandler@redhat.com
| const isUpdateAvailable = (...args: unknown[]): Promise => { |
|---|
| const eventArg = args.length > 0 ? args[0] : false; |
| if (typeof eventArg === 'boolean') { |
| return Promise.resolve(eventArg); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we can't use async keyword on the function ?
so we don't need to use Promise resolve but just returning the value ?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benoitf the type of the updater function in event-store is private updater: (...args: unknown[]) => Promise<T>, so if I don't do the promise resolve is gives me a type error
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if you set this function as being 'async' ?
Signed-off-by: Sonia Sandler ssandler@redhat.com