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

Update authorization step layout in onboarding flow #5049

Merged
merged 1 commit into from
Aug 5, 2021
Merged

Update authorization step layout in onboarding flow #5049

merged 1 commit into from
Aug 5, 2021

Conversation

gtsiolis
Copy link
Contributor

@gtsiolis gtsiolis commented Aug 4, 2021

Minor UI updates on the authorization step layout in onboarding flow.

BEFORE AFTER
auth-before auth-after
auth-dark-before auth-dark-after

@roboquat roboquat requested a review from JanKoehnlein August 4, 2021 09:20
@roboquat roboquat added the size/S label Aug 4, 2021
@gtsiolis gtsiolis removed the request for review from JanKoehnlein August 4, 2021 09:22
gtsiolis
gtsiolis commented Aug 4, 2021
View reviewed changes
components/dashboard/src/projects/NewProject.tsx
<span className="">
Install the GitHub app on selected Repositories to continue.
<span className="dark:text-gray-500">
Authorize GitHub (github.com) or select a different account.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

issue: Prompting for selecting a different account here would make more sense once we add the provider dropdown selection above this area. See specs in #4948.

components/dashboard/src/projects/NewProject.tsx
@@ -312,7 +313,7 @@ export default function NewProject() {

return (<div className="flex flex-col w-96 mt-24 mx-auto items-center">
<h1>New Project</h1>
<p className="text-gray-500 text-center text-base">Projects allow you to set up and acess Prebuilds.</p>
<p className="text-gray-500 text-center text-base">Select a git repository.</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

issue: This is missing the stepper component as seen in the specs in #4948. ● ○ ○

@gtsiolis
Copy link
Contributor Author

gtsiolis commented Aug 5, 2021 •
edited
Loading

@jankeromnes could you take a took a this small change? Otherwise, feel free to close this and pick up the changes in the project configurator updates (#4948). 🏀

/assign
/cc @jankeromnes

@roboquat roboquat requested a review from jankeromnes August 5, 2021 09:14
@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 5, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Hmm, the build seems to have failed:

Downloading rabbitmq from repo https://charts.bitnami.com/bitnami
Deleting outdated charts
Error: UPGRADE FAILED: another operation (install/upgrade/rollback) is in progress

Looks like a random intermittent problem. Re-triggering might fix.

/werft run

👍 started the job as gitpod-build-gt-update-auth-step-layout.2

@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 5, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Taking a look! 👓

jankeromnes
jankeromnes approved these changes Aug 5, 2021
View reviewed changes
Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Nailed it! 🎯 🔨 🎉

110% perfect, nothing for me to complain of even comment about. 😁 Also this looks so much better now! 😍 ✨

[Insert George-vs-PR meme here]

@roboquat roboquat added the lgtm label Aug 5, 2021
@roboquat
Copy link
Contributor

roboquat commented Aug 5, 2021

LGTM label has been added.

Git tree hash: c0f2d743d6b1d66b27f60862ce33e24a31404eea

@jankeromnes
Copy link
Contributor

The secret password to make @roboquat happy here is:

/approve no-issue

@roboquat
Copy link
Contributor

roboquat commented Aug 5, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jankeromnes

Associated issue requirement bypassed by: jankeromnes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 9e96ca7 into main Aug 5, 2021
@roboquat roboquat deleted the gt/update-auth-step-layout branch August 5, 2021 12:34
@gtsiolis
Copy link
Contributor Author

gtsiolis commented Aug 5, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Thanks @jankeromnes! 🏀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jankeromnes jankeromnes jankeromnes approved these changes

Assignees

@gtsiolis gtsiolis

@jankeromnes jankeromnes

Labels
approved component: dashboard size/S user experience
Projects
None yet
Milestone
No milestone
Development

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