-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add nationality to player #44
Conversation
1dcdc22
to
74d21e9
Compare
This is not the way to do it, please do not merge it. When I have time I'll explain in more detail the specs of this feature 🙏 |
I would like to understand better what you meant with the issue then, this is the relation that i understood 🤔 |
74d21e9
to
9ca8f34
Compare
The problem is not the interpretation of the requirements, is the solution in it itself |
What's the problem in the solution, could you review the code ? So i can find where the error is |
@MarioRodrigues10 We need to store nationalities as an atom with either a alpha2 or alpha3 standard. Afterwards we need to use this This also should come with a flag UI component implementation. |
@@ -1,2 +1,2 @@ | |||
erlang 25.0.2 | |||
elixir 1.13.4-otp-25 | |||
elixir 1.14.2-otp-25 |
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.
This should be a separate PR.
Its important to keep changes as short as possible and within the same domain
@@ -5,6 +5,8 @@ defmodule CesiumCupWeb.PlayerLive.FormComponent do | |||
alias CesiumCup.Teams | |||
|
|||
@extensions_whitelist ~w(.jpg .jpeg .gif .png) | |||
@nationalities File.read!("priv/fake/nationalities.txt") |> String.split("\n") | |||
@positions ~w(goalkeeper fixed wing target)a |
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.
This change is out of scope
"#{if @tab == "group" do | ||
"border-quinary text-gray-900" | ||
else | ||
"border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300" |
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.
This change is out of scope
No description provided.