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

Re-add creation failure error notifications to the environments page #3149

Merged
merged 2 commits into from
Jun 8, 2024

Conversation

bbonaby
Copy link
Contributor

@bbonaby bbonaby commented Jun 6, 2024

Summary of the pull request

I noticed when there is an error during the creation operation we don't tell the user in the latest preview and canary versions of Dev Home. Its due to our previous update to the landing page where we make all compute systems that are being created appear at the top of the list on the environments page.

To do that we added them to the top of the list of ComputeSystemCardBase items in the environments page. However, prior to that change we just added them to the end of the list and then had the last added item in the list subscribe to the environments landing page's OnComputeSystemOperationError event. So, when we did the change to add them at the top of the list we missed updating the line of code that also subscribes the newly added view model to the OnComputeSystemOperationError event. So, this change now makes sure that we subscribe to this event when the view model is created. Note: without subscribing to the event, the failure notification doesn't appear on the page.

Another thing I added was updating the text that appears in the card as the compute system is being created to show this failure as well.

Before changes:

notifications.when.there.was.an.error.creating.environment.-.before.mp4

After changes:

notifications.when.there.was.an.error.creating.environment.mp4

References and relevant issues

Detailed description of the pull request / Additional comments

Validation steps performed

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

@bbonaby bbonaby merged commit 99c8dc4 into main Jun 8, 2024
4 checks passed
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.

Environments page doesn't show creation error notifications
3 participants