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

🪟 🎨 Design fixes of the authentication page #16328

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

timroes
Copy link
Collaborator

@timroes timroes commented Sep 5, 2022

What

Fixes several design issues on the authentication page:

(1) The border of the google oauth button was gone (due to stylelint reordering the rules). Made sure this is no longer depending on rule ordering.

(2) Make the sign up page not being squished together when the screen height is small.
Previous behavior:
cloud airbyte io_signup
New behavior: (scrollable left side)
localhost_3000_login

(3) Hide the right side (in sync with Natalie), if the screen gets to small instead of pushing it below it. Since when the iframe is enabled for the right side, we have no good idea what a proper height for that should be for the content to render nicely.

@timroes timroes requested a review from a team as a code owner September 5, 2022 11:16
@github-actions github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Sep 5, 2022
flex-direction: column;
justify-content: space-between;
width: 100%;
display: none;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ By default now hide the right side on small screens.

Comment on lines -30 to -32
.container {
flex-direction: row;
height: 100%;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Moving those out of the media query was large part of the fix to get rid of the squishing behavior.

.container {
flex-direction: row;
height: 100%;
@media (min-width: 992px) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ I removed the height requirement, so make sure we're only using width, since I believe we still want to build responsive breakpoints on those width purely.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to set some breakpoints in _variables?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I do think we want so, but not as part of this PR. We should properly define and work with Nico on our responsive breakpoints, so this should be part of a separate PR with component. https://github.com/airbytehq/airbyte/issues/16357

@@ -45,6 +45,7 @@
.github {
background: #333;
color: colors.$white;
border: none;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Get rid of the border of the OAuth buttons being dependent on CSS rule ordering.

display: flex;
flex: 1 0 auto;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ This is the other part of making sure the primary area of the left side in the auth forms is actually scaling to all the place it can have, now using flexbox instead of some height calculation (that wasn't accurate).

@@ -7,12 +8,12 @@

.link {
text-decoration: none;
margin-top: variables.$spacing-2xl;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Add a bit more spacing above the GitHub button, and use the variable for it.


.links {
width: 100%;
display: flex;
flex-direction: row;
justify-content: flex-end;
margin-bottom: variables.$spacing-xl;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Give the top button (switch between login/signup) some padding to the bottom so the main content doesn't squish into it too much.

Copy link
Contributor

@letiescanciano letiescanciano left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@krishnaglick krishnaglick left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM!

@timroes timroes merged commit e593adc into master Sep 6, 2022
@timroes timroes deleted the tim/auth-page-design-fixes branch September 6, 2022 15:47
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants