Skip to content

Conversation

selfcontained
Copy link
Contributor

@selfcontained selfcontained commented May 23, 2023

Description

If you have the Cancel button focused in a ConfirmationModal and click Enter, instead of canceling it, it treats it as if you confirmed, which results in a delete behavior in many scenarios. This is due to the ref not getting forwarded correctly in the <Button> component.

Turns out you can't just a ref prop to a component, you need to wrap your component in forwardRef() and use the ref argument it passes in. More info in the docs.

Related Issue(s)

Fixes WEB-396, #16717

How to test

  • Go to your org SSO settings and create a fake SSO config. You can use https://accounts.google.com for the url, and w/e for the rest. Save it.
  • In the list view, click the overflow menu, and select delete.
  • The Cancel button should be focused by default. Hit Enter.
  • The config should not get deleted anymore now when you cancel.

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

Build Options

Build
  • /werft with-werft
    Run the build with werft instead of GHA
  • leeway-no-cache
  • /werft no-test
    Run Leeway with --dont-test
Publish
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer
  • analytics=segment
  • with-dedicated-emulation
  • with-ws-manager-mk2
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated
Preview Environment
  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

/hold

@selfcontained selfcontained marked this pull request as ready for review May 23, 2023 01:48
@selfcontained selfcontained requested a review from a team May 23, 2023 01:48
@selfcontained selfcontained requested a review from gtsiolis as a code owner May 23, 2023 01:48
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label May 23, 2023
@AlexTugarev
Copy link
Member

While testing this, I found another issue with accepting Create action on SSO modal, see WEB-397. Just verified, it's a pre-existing issue.

Copy link
Member

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

Works as advertised!

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up, @selfcontained! 🌮 🌮

There was a pending community contribution which attempted to resolve this in #17326, picked up in #17430 which will close in favor of this PR.

☝️ Cross-posting here for visibility and in case there are any changes worth picking up from there. Thanks again @RayAsh37 for helping!

This means, this will also fix https://github.com/gitpod-io/gitpod/issues/16717—updated PR description to include a reference to that issue, too. 🏓

@selfcontained
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit b1b699e into main May 23, 2023
@roboquat roboquat deleted the brad/web-396-clicking-enter-on-a-confirmationmodal branch May 23, 2023 14:12
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants