- Notifications
You must be signed in to change notification settings - Fork18
feat: style login redirect page - [ch1400]#221
Uh oh!
There was an error while loading.Please reload this page.
Conversation
rachelchhay left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LOVE the animation you added! I think it would look nice if the green banner was a circle instead. So kinda like a big start button. But I'll leave the main design feedback to Alex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM. Running./ci/steps/fmt.sh and./ci/steps/lint.sh should resolve the CI issues.
Uh oh!
There was an error while loading.Please reload this page.
@alexgilev let me know what you think, feel free to blow it up. |
This pull request has been linked toClubhouse Story #1400: coder login redirect page lacks any styling. |
@cmoog moved the template outside the control flow, also put it into a parameterized function after noticing another state needing to be expressed not specified in the ticket (a session_token missing). @rachelchhay put in your design suggestion (made it look like a power button) as well as lifted colors from our 2.0 scheme. Also notice the "off" state I added needed for a "session_token" missing state. 1400-on.mov1400-off.mov |
rachelchhay left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That's exactly what I had in mind. Looks great to me! Btw, Alex is on PTO today. I'm fine with merging as is and making changes after he has a chance to take a look.
internal/loginsrv/server.go Outdated
| func (srv *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) { | ||
| ctx := req.Context() | ||
| token := req.URL.Query().Get("session_token") | ||
| if token == "" { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| _, _ = fmt.Fprintf(w, "No session_token found.\n") // Best effort. | ||
| // _, _ = fmt.Fprintf(w, "No session_token found.\n") // Best effort. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| // _, _ = fmt.Fprintf(w, "No session_token found.\n") // Best effort. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed
@kvnlnt@rachelchhay@cmoog This is awesome stuff |
Adds basic styling to login redirect page.
1400-demo.mov
@vapurrmaid who else might I add as a reviewer?