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] Standardize PendingChangesDropdown component in workspaces list (replaces Modal) and stopped page (unchanged) #4078

Merged
merged 1 commit into from
Apr 29, 2021
Merged

[dashboard] Standardize PendingChangesDropdown component in workspaces list (replaces Modal) and stopped page (unchanged) #4078

merged 1 commit into from
Apr 29, 2021

Conversation

jankeromnes
Copy link
Contributor

Align with spec and side effect fix for dark theme:

image (2)

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 27, 2021 •
edited
Loading

Currently looks like this:

Screenshot 2021-04-27 at 14 03 34 Screenshot 2021-04-27 at 14 03 50

Problems to fix:

  • Vertical alignment is off
  • Clicking on it doesn't work

@jankeromnes jankeromnes changed the title [dashboard] Replace workspaces list changes modal with changes dropdown from stopped page [dashboard] Standardize PendingChangesDropDown component in workspaces list (replaces Modal) and stopped page (unchanged) Apr 27, 2021
@jankeromnes jankeromnes changed the title [dashboard] Standardize PendingChangesDropDown component in workspaces list (replaces Modal) and stopped page (unchanged) [dashboard] Standardize PendingChangesDropdown component in workspaces list (replaces Modal) and stopped page (unchanged) Apr 27, 2021
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 27, 2021 •
edited
Loading

Now looks like this:

Screenshot 2021-04-27 at 14 25 01 Screenshot 2021-04-27 at 14 25 17
Screenshot 2021-04-27 at 14 56 10 Screenshot 2021-04-27 at 14 56 26
Screenshot 2021-04-27 at 14 25 45 Screenshot 2021-04-27 at 14 25 29

@jankeromnes jankeromnes force-pushed the jx/workspaces-changes-dropdown branch from 6fffb99 to 8295c2c Compare April 27, 2021 12:57
@jankeromnes jankeromnes marked this pull request as ready for review April 27, 2021 12:57
@jankeromnes
[dashboard] Standardize PendingChangesDropdown component in workspace…
58f2215 
…s list (replaces Modal) and stopped page (unchanged)
@jankeromnes jankeromnes force-pushed the jx/workspaces-changes-dropdown branch from 8295c2c to 58f2215 Compare April 27, 2021 13:35
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 27, 2021 •
edited
Loading

@gtsiolis could you please take a look, and make a final decision? 🙏

  • Either we go with this PR, replacing the Changes Modal with a dropdown

  • Or we temporarily park this PR to keep the Changes Modal, and simply adjust the balloon colors for dark theme, and fix the Modal-induced scrollbar

@jankeromnes jankeromnes requested a review from gtsiolis April 27, 2021 14:38
@gtsiolis
Copy link
Contributor

gtsiolis commented Apr 28, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-jx-workspaces-changes-dropdown.3

@gtsiolis
Copy link
Contributor

gtsiolis commented Apr 28, 2021 •
edited
Loading

Looking at this now! 👀 Postponed, will take a look in an hour or so.

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 28, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Probably not critical for the release tomorrow (since dark theme is soft-launch, and there will probably be other small things to fix here and there 😇).

So: no rush 🌴

gtsiolis
gtsiolis approved these changes Apr 28, 2021
View reviewed changes
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 aligning this @jankeromnes! UX looks good! 🌟

Alright, I think this looks good to merge as is! We could probably add some hover and focus states on this new changes dropdown but this is certainly out of the scope of this PR.

gtsiolis
gtsiolis reviewed Apr 28, 2021
View reviewed changes
components/dashboard/src/workspaces/WorkspaceEntry.tsx
message: string, items: string[]
}

export function getPendingChanges(wsi?: WorkspaceInstance): PendingChanges[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Deleted code is the best code! 💯

@gtsiolis
Copy link
Contributor

gtsiolis commented Apr 28, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Either we go with this PR, replacing the Changes Modal with a dropdown
...

Sounds like a good plane for now! 🗺️

Posting this here for future reference:

Also, this seems like a good two-way door decision which allows us to come back and reverse this change in the future! 🔮

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 29, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Awesome, many thanks for the review! Then let's go forward with unifying this, but I'm more than happy to re-introduce a Changes Modal later. 🚀

@jankeromnes jankeromnes merged commit 550bb79 into main Apr 29, 2021
@jankeromnes jankeromnes deleted the jx/workspaces-changes-dropdown branch April 29, 2021 05:18
@gtsiolis gtsiolis mentioned this pull request Apr 29, 2021
Closed
@jankeromnes jankeromnes mentioned this pull request May 10, 2021
Closed
@gtsiolis gtsiolis mentioned this pull request May 17, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtsiolis gtsiolis gtsiolis approved these changes

Assignees
No one assigned
Labels
component: dashboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jankeromnes @gtsiolis