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.

Sign up for GitHub

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

Jump to bottom

integration login #45

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

integration login #45

wants to merge 2 commits into from

Conversation

sludovicdelys
Copy link
Contributor

Aujourd'hui nous avons créer un petit login :-)

@sludovicdelys sludovicdelys self-assigned this Aug 17, 2021
ameliedefrance
ameliedefrance approved these changes Aug 17, 2021
View reviewed changes
Copy link
Contributor

@ameliedefrance ameliedefrance left a comment

Choose a reason for hiding this comment

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

Perfect ✨

@ogizanagi
Copy link
Member

ogizanagi commented Aug 17, 2021 •
edited
Loading

Le deploy a échoué, mais pas de panique. On va passer en Node 16 pour s'éviter ce problème :)

Tu peux revert le fichier demo/package-lock.json de cette PR en attendant ?

@sludovicdelys
Copy link
Contributor Author

sludovicdelys commented Aug 17, 2021 via email

@ogizanagi
Copy link
Member

Le revert du fichier en question, oui

github-actions bot pushed a commit that referenced this pull request Aug 17, 2021
@sludovicdelys
[AUTO] Deploy PR #45
7007562 
for commit 4df3bb2
github-actions bot pushed a commit that referenced this pull request Aug 17, 2021
@sludovicdelys
[AUTO] Deploy PR #45
1439bf1 
for commit bb5fd75
github-actions bot pushed a commit that referenced this pull request Aug 17, 2021
@sludovicdelys
[AUTO] Deploy PR #45
cb7db5e 
for commit 188001f
ogizanagi
ogizanagi reviewed Aug 17, 2021
View reviewed changes
demo/templates/demo/components.html.twig
</button>
</div>

<div class="login">
Copy link
Member

Choose a reason for hiding this comment

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

Plutôt que de le mettre au milieu des composants, est-ce que ça ne vaudrait pas une full page dédiée ?
D'autant que outre le formulaire de connexion, d'avantage de mise en page peut être souhaitable pour être adapté à une page entière.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes je suis d'accord, et plus globalement on doit refaire une passe sur le découpage de la démo. On va avoir besoin de créer des pages thématiques (issue ici #36) et le login pourra filer sur sa propre page à ce moment là ?

Copy link
Contributor

Choose a reason for hiding this comment

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

À la limite ça peut se faire dans une autre PR il faudra qu'on se pose notamment avec Christelle sur ce qu'on veut présenter / comment on veut le présenter

Copy link
Contributor

Choose a reason for hiding this comment

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

Ça réouvre le sujet Storybook 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ameliedefrance ameliedefrance ameliedefrance approved these changes

@ogizanagi ogizanagi ogizanagi left review comments

Assignees

@sludovicdelys sludovicdelys

Labels
Intégration 📐 R2 Review 👀 Ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sludovicdelys @ogizanagi @ameliedefrance