Conversation
EdroCode
requested review from
AfonsoMartins26,
enricoprazeres,
filipapeixoto25 and
nunom27
|
Closes #28 |
enricoprazeres
left a comment
enricoprazeres
left a comment
There was a problem hiding this comment.
Awesome job! 🔥 Just need you to polish some things.
| ~H""" | ||
| <div> | ||
| <.form for={@form} phx-change="validate" phx-target={@myself}> | ||
| <div class="mb-2 mt-4"> |
There was a problem hiding this comment.
Avoid using these margins across the code. You should prefer using flex and gaps. It makes the code cleaner and less unpredictable.
| </span> | ||
| </div> | ||
| <% else %> | ||
| <div class="grid w-full h-full grid-cols-[auto_1rem] justify-stretch py-1 pr-2"> |
There was a problem hiding this comment.
This grid and notation usage is not necessary if the suggestions are followed (mainly the one on the core_components -> see it). Also, consider removing all occurrences as well.
| def handle_event("validate", %{"auth" => form}, socket) do | ||
| {:noreply, | ||
| socket | ||
| |> assign( | ||
| form: | ||
| FormData.to_form( | ||
| %{"password" => form["password"], "confirm_password" => form["confirm_password"]}, | ||
| as: :auth | ||
| ) | ||
| )} | ||
| end |
There was a problem hiding this comment.
With the changeset you validate the data you're submitting in the form. It's strongly desired to use it.
| def handle_event("validate", %{"auth" => form}, socket) do | |
| {:noreply, | |
| socket | |
| |> assign( | |
| form: | |
| FormData.to_form( | |
| %{"password" => form["password"], "confirm_password" => form["confirm_password"]}, | |
| as: :auth | |
| ) | |
| )} | |
| end | |
| @impl true | |
| def handle_event("validate", %{"auth" => form}, socket) do | |
| changeset = %User{} |> User.changeset(form) | |
| {:noreply, | |
| socket | |
| |> assign(form: to_form(changeset, as: "auth"))} | |
| end |
| @@ -0,0 +1,26 @@ | |||
| defmodule Yearbook.User do | |||
| @moduledoc """ | |||
There was a problem hiding this comment.
Maybe add validations for the email and to see if the password and confirm password fields are equal. With this, you should make alerts at the form.
| <div class="bg-white/30 m-4 w-auto backdrop-blur-md p-3 rounded-2xl hover:scale-102 transition-transform hover:bg-white/50"> | ||
| <a href="/" class="flex items-center gap-3 cursor-pointer"> |
There was a problem hiding this comment.
| <div class="bg-white/30 m-4 w-auto backdrop-blur-md p-3 rounded-2xl hover:scale-102 transition-transform hover:bg-white/50"> | |
| <a href="/" class="flex items-center gap-3 cursor-pointer"> | |
| <div class="bg-white/30 m-4 w-auto backdrop-blur-md rounded-2xl hover:scale-102 transition-transform hover:bg-white/50"> | |
| <a href="/" class="flex items-center gap-3 cursor-pointer p-3"> |
There was a problem hiding this comment.
Maybe refactor this structure. Since there is just one background, just remove the folder and put it inside the images folder directly.
2 participants
Closes #32
Closes #30
Closes #31