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

Intégration de Place des entreprises #2034

Merged
merged 22 commits into from
Mar 15, 2022

Conversation

wiinxt
Copy link
Contributor

@wiinxt wiinxt commented Mar 2, 2022

@wiinxt wiinxt force-pushed the #1286-add-places-des-entreprises-iframe branch from 724c69b to 7748342 Compare March 2, 2022 15:37
@github-actions
Copy link

github-actions bot commented Mar 2, 2022

🚀 La branche est déployée !

@wiinxt wiinxt force-pushed the #1286-add-places-des-entreprises-iframe branch 3 times, most recently from 652f274 to 1bac61f Compare March 3, 2022 18:34
site/source/pages/Gerer/Home.tsx Outdated Show resolved Hide resolved
site/source/components/PlaceDesEntreprises.tsx Outdated Show resolved Hide resolved
@wiinxt wiinxt force-pushed the #1286-add-places-des-entreprises-iframe branch from 5b3bdbc to be6e0d3 Compare March 9, 2022 12:34
@wiinxt wiinxt marked this pull request as ready for review March 9, 2022 13:44
@wiinxt
Copy link
Contributor Author

wiinxt commented Mar 9, 2022

Il ne manque que le pré remplissage du siret.

@wiinxt wiinxt force-pushed the #1286-add-places-des-entreprises-iframe branch from 7bcfcbc to 4b14a27 Compare March 10, 2022 10:06
* Inspired from this issue https://github.com/adobe/react-spectrum/issues/1513
* @param buttonRef Ref of the button
*/
export const usePreventClickAfterTouch = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Trop bien d'avoir corrigé ce bug particulièrement embêtant ! C'est étonnant qu'il ne soit pas corrigé côté react-spectrum...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Il me semble qu'ils sont en train de le corriger, à voir dans les prochaines version adobe/react-spectrum#1513


return (
<IframeContainer>
<Iframe
Copy link
Contributor

Choose a reason for hiding this comment

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

Pas bloquant pour merger mais on a le même problème que sur notre intégration iframe : quand on clique sur un lien à l'intérieur de l'iframe, la position de scroll ne remonte pas en haut de la “page” #1348

Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'ai essayé un fix pour scroll to top sur l'onLoad de l'iframe, j'attend le build pour vérifier que ça fonctionne bien

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ça fonctionne bien !

@@ -199,7 +199,7 @@ jobs:

- mon-entreprise : ${{ needs.deploy-context.outputs.fr_url }}
- mycompanyinfrance : ${{ needs.deploy-context.outputs.en_url }}
- storybook : ${{ needs.deploy-context.outputs.fr_url }}/dev/storybook/ or ${{ needs.deploy-context.outputs.en_url }}/dev/storybook/
- storybook : ${{ needs.deploy-context.outputs.fr_url }}/dev/storybook/ ([version EN](${{ needs.deploy-context.outputs.en_url }}/dev/storybook/))
Copy link
Contributor

Choose a reason for hiding this comment

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

C'est peut-être possible de gérer les versions françaises et anglaises dans le même Storybook ? (pareil évidemment pas bloquant)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enfaite les deux lien mène au même Storybook, on pourrait juste supprimer le deuxième lien

@wiinxt wiinxt force-pushed the #1286-add-places-des-entreprises-iframe branch from 56eb238 to 67a97a5 Compare March 14, 2022 11:34
@wiinxt
Copy link
Contributor Author

wiinxt commented Mar 14, 2022

La pr est prête mais le siret ne se prérempli pas encore (ça fonctionne avec un lien direct mais pas dans l'iframe je ne sais pas pourquoi)

@johangirod
Copy link
Collaborator

Petit problème d'affichage avec la nouvelle modale sur desktop : elle s'ouvre trop en bas

Peek.14-03-2022.15-44.mp4

@wiinxt
Copy link
Contributor Author

wiinxt commented Mar 15, 2022

La pr est prête mais le siret ne se prérempli pas encore (ça fonctionne avec un lien direct mais pas dans l'iframe je ne sais pas pourquoi)

Vue avec PdE, ça vient de leur coté

@wiinxt wiinxt merged commit 078e60b into master Mar 15, 2022
@wiinxt wiinxt deleted the #1286-add-places-des-entreprises-iframe branch March 15, 2022 12:12
johangirod added a commit that referenced this pull request Mar 15, 2022
wiinxt added a commit that referenced this pull request Mar 16, 2022
wiinxt added a commit that referenced this pull request Mar 16, 2022
@mquandalle mquandalle changed the title WIP Place des entreprises Intégration de Place des entreprises May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants