crc-org / crc-extension

Red Hat OpenShift Local Extension for integration of OpenShift Local clusters with Podman Desktop
Apache License 2.0
17 stars 20 forks source link

Cancelling installation of the extension must not trigger the download #81

Open slemeur opened 1 year ago

slemeur commented 1 year ago

When installing from a fresh environment. When the popup is asking to choose a preset, if the user presses "ESC3 on the keyboard, the download process still start.

gbraad commented 1 year ago

@slemeur macOS ? Windows ? It shouldn't matter, as this is a lack of modality in the UI. We should be able to receive the exit without a selection (cancel), but will have a look. We had fixed it before ... :-/

slemeur commented 1 year ago

MacOS. on my side

gbraad commented 1 year ago

According to:

https://github.com/crc-org/crc-extension/blob/9d20e0e0fa843ed0187c22a743b46b3c3aae674a/src/crc-setup.ts#L44-L48

this shows a popup/notification

image

Maybe on macOS this notification is not shown very visibly. I would have to ask someone else to verify this...

But, would the suggestion be instead to just 'break' the initialization/installation flow? I think the problem in that case is that we don't know it is an initialization flow when people restart. The default preset will still be used (and the user is not pushed to choosing a preset). Preferably this dialog is modal and does not allow a break: https://github.com/containers/podman-desktop/issues/1992

gbraad commented 1 year ago

If we return

    if (!preset) {
      return false;
    } else {

instead, we can cancel out of the flow. But the Podman Desktop user interface will NOT be updated and still shows 'Install'.

If the users decide to do this again, the 'download' will happen from a cached version, will perform the install again as upgrade and ask the same question.

It is here more a question of what we want to do:

I think at this point we can go or the 'default option' as implemented and verify if the dialog was shown.

@anjannath can you try this on a macBook? To get into this state, you have to remove the CRC installation before starting PD, otherwise the init will not be triggered.

anjannath commented 1 year ago

Tried it on macOS, and notification is shown that its going to use the default bundle , but as per macOS settings you have to first "give permissions to show notifications" we should probably ask for this permission well before we have to show the first notification.

image
slemeur commented 1 year ago

Sorry, but this is not a question it's a bug

gbraad commented 1 year ago

Sorry, but this is not a question it's a bug

I added the label aswe already handle the situation by informing the user, but have an open question related to expectation. Please see https://github.com/crc-org/crc-extension/issues/81#issuecomment-1535683603 for options

I'll add need more information instead