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

Improvement : main UI refacto #45

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

joshuaVayer
Copy link
Collaborator

Refacto global sur les éléments visuels

@joshuaVayer joshuaVayer requested a review from HNTQ January 15, 2022 13:32
@joshuaVayer joshuaVayer self-assigned this Jan 15, 2022
@HNTQ
Copy link
Owner

HNTQ commented Jan 15, 2022

Dans le bandeau Movie search est en noir sur fond noir
|image|image|

@HNTQ
Copy link
Owner

HNTQ commented Jan 15, 2022

Est ce qu'on rajoute sur cette PR le nombre de caractère pour s'enregistrer?
image

@HNTQ
Copy link
Owner

HNTQ commented Jan 15, 2022

Image des acteurs à bord carré et container à angle arrondi. je le note mais ne fait pas parti de tes modifs
image

@HNTQ
Copy link
Owner

HNTQ commented Jan 15, 2022

Pourquoi ajouter un yarn.lock ? vis à vis de tailwind?

medias[filter] = parse_query_by_keyword(query, filter)[filter]
else:
for media in medias:
query = query_by_keyword(title, media)
if page :
query = query_by_keyword(title, media, page)
Copy link
Owner

@HNTQ HNTQ Jan 15, 2022

Choose a reason for hiding this comment

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

ici tu anticipes la suite? avec page

src=https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcS9X22avAKEnosxlprQD89neJKvIafesu-5cA&usqp=CAU"
alt="Search Movie">
src=https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcS9X22avAKEnosxlprQD89neJKvIafesu-5cA&usqp=CAU"
alt="Search Movie">
<span class="ml-4 text-xl font-bold uppercase">Movie Search</span>
Copy link
Owner

@HNTQ HNTQ Jan 15, 2022

Choose a reason for hiding this comment

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

ne pas oublier le texte Movie Search qui est resté en noir

<input id="movie" type="checkbox" name="filter" value="movie" >
<label for="tv">Series</label>
<label class="px-1 hover:underline cursor-pointer" for="movie">Movies</label>
Copy link
Owner

Choose a reason for hiding this comment

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

pour ce genre de cas, peut être passer par des macros à l'avenir afin de pouvoir modifier le design une seule fois à un seul endroit

@@ -18,7 +18,7 @@
{% endif %}

{{ button_primary(type="submit", text="Log In", class="mt-5 mb-3") }}
{{ nav_link_secondary_button(endpoint="/register", text="Create an account", class="mb-5 text-center") }}
{{ link_secondary_button(endpoint="/register", text="Create an account", class="mb-5 text-center") }}
{{ nav_link(text="Forgot your password ?", endpoint="#", class="text-center") }}
</form>
Copy link
Owner

Choose a reason for hiding this comment

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

Forgot your password est resté en blanc sur fond blanc

@@ -18,7 +18,7 @@
{% endif %}

{{ button_primary(type="submit", text="Log In", class="mt-5 mb-3") }}
{{ nav_link_secondary_button(endpoint="/register", text="Create an account", class="mb-5 text-center") }}
{{ link_secondary_button(endpoint="/register", text="Create an account", class="mb-5 text-center") }}
Copy link
Owner

Choose a reason for hiding this comment

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

dans ce cas on utilise une macro Button pour le log-in et une macro link pour Create an account
j'enleverais le soulignage du texte au hover pour les deux boutons

<!--
3 - Button types
-->
{% macro link_primary_button(endpoint, text, class) %}
Copy link
Owner

Choose a reason for hiding this comment

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

on a nav_link_primary_button et link_primary_button doublon?

</div>
{% endmacro %}

{% macro card_media(link, title, poster, overview, date, rate, class) %}
Copy link
Owner

Choose a reason for hiding this comment

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

Pourquoi conserver card_search?
renommer card_search_people en card_people ou card_media_people?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants