Skip to content

[dashboard] Re-implement Plans page for new dashboard #3550

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
Merged
merged 5 commits into from
Mar 31, 2021
Merged

[dashboard] Re-implement Plans page for new dashboard #3550

merged 5 commits into from
Mar 31, 2021

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Mar 23, 2021 •
edited
Loading

Fixes #3358

TODO:

  • Migrate plans.ts from gitpod-com to gitpod-io
  • Display available plans in Plans
  • Add €/$ toggle when relevant
  • Fetch user data, show current plan and remaining hours
  • Implement Upgrade / Downgrade modals
  • Implement "pending upgrade", "pending downgrade" and "scheduled downgrade" states
  • Actively poll server to quickly resolve any "pending" states
  • Add missing content to Upgrade / Downgrade modals
  • Show assigned Team Plan seats if any (stretch goal)
  • Implement "cancel downgrade" (stretch goal)

@jankeromnes jankeromnes marked this pull request as draft March 23, 2021 07:11
@jankeromnes jankeromnes force-pushed the jx/plans branch 2 times, most recently from 9c4ee93 to 55364d3 Compare March 23, 2021 08:12
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Mar 23, 2021 •
edited
Loading

Currently looks like this:

Screenshot 2021-03-30 at 15 31 04

Screenshot 2021-03-30 at 16 12 21

@jankeromnes jankeromnes force-pushed the jx/plans branch 4 times, most recently from bb08e33 to 1864cf7 Compare March 26, 2021 10:35
@jankeromnes jankeromnes changed the title [dashboard] Re-implement Plans page in new dashboard [dashboard] Re-implement Plans page for new dashboard Mar 26, 2021
@jankeromnes jankeromnes force-pushed the jx/plans branch 9 times, most recently from 9db5de9 to 9c5ef33 Compare March 30, 2021 12:03
@jankeromnes jankeromnes marked this pull request as ready for review March 30, 2021 12:04
@gtsiolis gtsiolis added this to the March 2021 milestone Mar 30, 2021
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Mar 30, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Ready for review! Please take a look @svenefftinge @AlexTugarev @gtsiolis 🙏

(Note, the gitpod-com deployment works, this one here won't work.)

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 30, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Looking at this now! 👀

gtsiolis
gtsiolis suggested changes Mar 30, 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.

Here's the plan! This looks great @jankeromnes! Left some comments. 🗺️

components/dashboard/src/settings/Plans.tsx Outdated
</div>
</div>
<div className="flex justify-end mt-6">
<button onClick={doUpgrade}>Upgrade</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

question: What do you think of introducing a secondary button variant for the confirm (green color) action? Using the following attributes could could suffice. Feel free to add this inline here.

text-green-600 bg-green-100 hover:text-green-600 hover:bg-green-300

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, thanks! Do you think this should be added to our current centralized button variants?

gitpod/components/dashboard/src/index.css

Lines 36 to 51 in ec62ded

@layer components {
button {
@apply cursor-pointer px-4 py-2 my-auto bg-green-600 hover:bg-green-700 text-gray-100 text-sm font-medium rounded-md focus:outline-none focus:ring transition ease-in-out;
}
button.secondary {
@apply bg-gray-100 hover:bg-gray-200 text-gray-500 hover:text-gray-600;
}
button.danger {
@apply bg-red-600 hover:bg-red-700 text-gray-100;
}
button.danger.secondary {
@apply bg-red-50 hover:bg-red-100 text-red-600 hover:text-red-700;
}
button:disabled {
@apply cursor-default opacity-50 pointer-events-none;
}

Copy link
Contributor

@gtsiolis gtsiolis Mar 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Ideally yes, but this will become more clear on the way as we split components down to smaller pieces and the need for component hierarchy, variants, etc becomes obvious for everyone one the team. I'd say both approaches are completely fine here for now! 🔥

AlexTugarev
AlexTugarev reviewed Mar 31, 2021
View reviewed changes
@svenefftinge
Copy link
Member

svenefftinge commented Mar 31, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-jx-plans.18

@jankeromnes jankeromnes force-pushed the jx/plans branch 3 times, most recently from 0f35b07 to 7102e2b Compare March 31, 2021 11:54
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Mar 31, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-jx-plans.23

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Mar 31, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-jx-plans.24

AlexTugarev
AlexTugarev reviewed Mar 31, 2021
View reviewed changes
AlexTugarev
AlexTugarev reviewed Mar 31, 2021
View reviewed changes
@jankeromnes jankeromnes force-pushed the jx/plans branch 2 times, most recently from 1198a94 to 5d7e2ee Compare March 31, 2021 13:41
@AlexTugarev
Copy link
Member

@jankeromnes could you center the cards?

2021-03-31 17 35 15

@AlexTugarev
Copy link
Member

@jankeromnes, the cards could use more width, no? I count at least 5 ellipsis.

Screen Shot 2021-03-31 at 17 37 49

gtsiolis
gtsiolis approved these changes Mar 31, 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.

Let's center the cards as @AlexTugarev mentioned in #3550 (comment) and merge this! 🚢

@jankeromnes jankeromnes merged commit cb9eb44 into main Mar 31, 2021
@jankeromnes jankeromnes deleted the jx/plans branch March 31, 2021 16:59
jankeromnes added a commit that referenced this pull request Mar 31, 2021
@jankeromnes
[dashboard] Don't load all images from one module
62cb8b4 
Fixes #3550 (comment)
@jankeromnes jankeromnes mentioned this pull request Apr 7, 2021
Closed
pavan-tri pushed a commit to trilogy-group/gitpod that referenced this pull request Apr 28, 2021
@jankeromnes
[dashboard] Don't load all images from one module
6b4e69a 
Fixes gitpod-io#3550 (comment)
@gtsiolis gtsiolis mentioned this pull request May 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@svenefftinge svenefftinge svenefftinge left review comments

@AlexTugarev AlexTugarev AlexTugarev left review comments

@gtsiolis gtsiolis gtsiolis approved these changes

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

Successfully merging this pull request may close these issues.

[Dashboard] Plans Page
4 participants
@jankeromnes @gtsiolis @svenefftinge @AlexTugarev