Link new UI to auth and backend #36
Conversation
…ary task. This also migrates some pages to properly link up authorization flows. Some cruft is still in
…them in the auth signin page
… radio group for the rating scale
fozziethebeat
requested review from
yk and
k-nearest-neighbor
and removed request for
yk
…on to make it easier to do email based login. Also adding env variables that work by default with the docker services
There was a problem hiding this comment.
This looks great.
While I'm still working on getting this fully tested on local. I'm turning in for the night and don't want to hold you up.
I've looked at the code and it all makes sense to me.
Stuff we can work on soon
- instructions for running local
- auth workflows
There was a problem hiding this comment.
clear to merge, but I left a few comments / questions.
Also, from what I remember, using process.env directly leads to problems in docker builds, as variables are captured at compile time, not at runtime. Next has a runtime configuration framework here: https://nextjs.org/docs/api-reference/next.config.js/runtime-configuration . I suggest we use that.
| image: postgres | ||
| restart: always | ||
| ports: | ||
| - 5433:5432 | ||
| environment: | ||
| POSTGRES_USER: postgres | ||
| POSTGRES_PASSWORD: postgres | ||
| healthcheck: | ||
| test: ["CMD", "pg_isready", "-U", "postgres"] | ||
| interval: 2s | ||
| timeout: 2s | ||
| retries: 10 |
There was a problem hiding this comment.
| image: postgres | |
| restart: always | |
| ports: | |
| - 5433:5432 | |
| environment: | |
| POSTGRES_USER: postgres | |
| POSTGRES_PASSWORD: postgres | |
| healthcheck: | |
| test: ["CMD", "pg_isready", "-U", "postgres"] | |
| interval: 2s | |
| timeout: 2s | |
| retries: 10 | |
| extends: db |
There was a problem hiding this comment.
I was intentionally dropping extends here to improve readability rather than reduce code. I especially wanted to expose the port and user/password details since they're used below.
That for me at least makes it a little easier to see everything happening without having to check another docker compose file.
|
|
||
| # This fakes and SMTP email server. User registration emails can be found by going to | ||
| # localhost:1080 and opening the emails listed. | ||
| maildev: |
There was a problem hiding this comment.
is the mail server needed just for the custom auth or also for the discord auth workflow?
There was a problem hiding this comment.
Yes it's just for the custom auth.
I'll enhance the comment in the next PR.
| <button | ||
| type="button" | ||
| className="inline-flex items-center rounded-md border border-gray-300 bg-white px-4 py-2 text-sm font-medium text-gray-700 shadow-sm hover:bg-gray-50 focus:outline-none focus:ring-2 focus:ring-indigo-500 focus:ring-offset-2" | ||
| > | ||
| Log in | ||
| </button> |
There was a problem hiding this comment.
button looks exactly the same as the button above, could this be a component?
There was a problem hiding this comment.
I agree, this was originally from @k-nearest-neighbor. I'm leaving css and component cleanup for later changes.
| <button | ||
| type="button" | ||
| onClick={() => submitResponse(tasks[0])} | ||
| className="nline-flex items-center rounded-md border border-transparent bg-indigo-600 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500 focus:ring-offset-2" |
There was a problem hiding this comment.
should this be inline-flex instead of nline-flex?
There was a problem hiding this comment.
I didn't catch this either in my first review. It probably should be. Will be fixed later.
|
Agreed on using an env setup that works well with Docker. That's what i'll be exploring in the next PR. |
This integrates the recent new landing page and demo grading page with full user authorization and full task fetching & updating.
included in this is use of Chakra-UI components where they make sense. The initial uses are:
Later we can adopt more Chakra components as desired while still use tailwindcss