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.

Sign up for GitHub

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

Jump to bottom

[dashboard] Downtime in-app announcements #18761

Merged
merged 7 commits into from
Sep 21, 2023
Merged

[dashboard] Downtime in-app announcements #18761

merged 7 commits into from
Sep 21, 2023

Conversation

geropl
Copy link
Member

@geropl geropl commented Sep 20, 2023 •
edited
Loading

Description

Adds a notification on dashboard to announce our planned downtime:
image

Summary generated by Copilot

🤖 Generated by Copilot at 168e3ce

This pull request adds a new component AppNotifications that can display notifications to the user across the app. It also adds a feature flag scheduled_downtime_notification that controls whether to show a notification about a planned maintenance. The feature flag is disabled by default.

Related Issue(s)

Fixes EXP-664

How to test

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
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
  • /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. If enabled, with-preview and with-large-vm will be enabled.
  • with-monitoring

/hold

@geropl geropl force-pushed the gpl/db-notification branch from 168e3ce to c757de3 Compare September 20, 2023 15:18
@geropl geropl marked this pull request as ready for review September 20, 2023 15:20
@geropl geropl requested a review from a team as a code owner September 20, 2023 15:20
gtsiolis
gtsiolis approved these changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@gtsiolis gtsiolis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

UX LGTM. 🏁

Left two minor non-blocking comments on the copy and the alert layout.

components/dashboard/src/AppNotifications.tsx Outdated Show resolved Hide resolved
gtsiolis
gtsiolis reviewed Sep 20, 2023
View reviewed changes
components/dashboard/src/components/Alert.tsx
<button
type="button"
className="bg-transparent p-1"
<Button
Copy link
Contributor

@gtsiolis gtsiolis Sep 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

issue(non-blocking): The button component here is causing a minor layout shift. Can be visible on large viewports or when the message in the alert is wrapper in a single line.

CURRENT Alert component specs
Frame 1908 Frame 1909

@mbrevoort
Copy link
Contributor

Please update the copy to be:

Gitpod will be uncharacteristically unavailable for essential maintenance beginning {localized time} and lasting up to one hour. We apologize for any inconvenience this may cause.

Also, I presume linking to this StatusPage update is a placeholder? The alert is from 2022.

@geropl geropl force-pushed the gpl/db-notification branch from c757de3 to 964904d Compare September 20, 2023 20:25
@geropl
Copy link
Member Author

geropl commented Sep 20, 2023

Please update the copy to be:

✔️

Also, I presume linking to this StatusPage update is a placeholder? The alert is from 2022.

Sure. I used this as descriptive placeholder before creating the new entry tomorrow. 👍

@geropl
Copy link
Member Author

geropl commented Sep 20, 2023

Thanks for all the input! 🙏

Will merge the PR as-is tomorrow morning after creating a new statuspage entry and linking it here.

@geropl
Copy link
Member Author

geropl commented Sep 21, 2023

/unhold

@roboquat roboquat merged commit cebee56 into main Sep 21, 2023
14 checks passed
@roboquat roboquat deleted the gpl/db-notification branch September 21, 2023 09:15
geropl added a commit that referenced this pull request Sep 25, 2023
@geropl
Revert "[dashboard] Downtime in-app announcements (#18761)"
1a5845a 
This reverts commit cebee56.
geropl added a commit that referenced this pull request Sep 25, 2023
@geropl
Revert "[dashboard] Downtime in-app announcements (#18761)"
8fd4c2a 
This reverts commit cebee56, modulo some fixes we want to keep.
roboquat pushed a commit that referenced this pull request Sep 25, 2023
@geropl
Revert "[dashboard] Downtime in-app announcements (#18761)" (#18789)
ba79893 
This reverts commit cebee56, modulo some fixes we want to keep.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@filiptronicek filiptronicek filiptronicek left review comments

@gtsiolis gtsiolis gtsiolis approved these changes

@mbrevoort mbrevoort mbrevoort left review comments

@selfcontained selfcontained selfcontained approved these changes

Assignees
No one assigned
Labels
size/L team: team-experience
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@geropl @mbrevoort @gtsiolis @selfcontained @filiptronicek @roboquat