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

front: split manchette with div #899

Merged
merged 1 commit into from
Feb 27, 2025
Merged

front: split manchette with div #899

merged 1 commit into from
Feb 27, 2025

Conversation

theocrsb
Copy link
Contributor

@theocrsb theocrsb force-pushed the t/update-manchette-props branch from 56ac5f7 to be29ae9 Compare February 12, 2025 15:05
@Akctarus Akctarus self-assigned this Feb 12, 2025
@Akctarus Akctarus force-pushed the t/update-manchette-props branch from c90f979 to aadf890 Compare February 17, 2025 15:03
@theocrsb theocrsb requested a review from clarani February 19, 2025 12:31
@theocrsb theocrsb force-pushed the t/update-manchette-props branch 3 times, most recently from 38f7953 to c5c4899 Compare February 19, 2025 13:57
@Akctarus Akctarus force-pushed the t/update-manchette-props branch from c5c4899 to 78d882a Compare February 20, 2025 10:01
@Akctarus
Copy link
Contributor

Thanks for your comments, we've taken them on board and fixed all the problems

@Akctarus Akctarus requested a review from jacomyal February 20, 2025 10:04
@theocrsb theocrsb marked this pull request as ready for review February 20, 2025 13:27
@theocrsb theocrsb requested a review from a team as a code owner February 20, 2025 13:27
Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

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

Thank you for this PR.
Some differences with the sketch.
Storybook:
Screenshot From 2025-02-24 14-09-27

Sketch:
Screenshot From 2025-02-24 14-08-11

  • The gov must take 100% of the width
  • The borders are different at the top and bottom of the split part

@Akctarus
Copy link
Contributor

Akctarus commented Feb 24, 2025

Thank you for this PR. Some differences with the sketch. Storybook: Screenshot From 2025-02-24 14-09-27

Sketch: Screenshot From 2025-02-24 14-08-11

  • The gov must take 100% of the width
  • The borders are different at the top and bottom of the split part

Thank you for your comment ! The aim of this PR is to integrate a component (any component) into the manchette. The “clean” integration of the gov will take place in a second PR, when this PR and that of the GET split will also be merged.

Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

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

I don't think this AC has not been processed.
"dans la story, l'utilisateur peut préciser la taille de l'élément et sa position"

@theocrsb
Copy link
Contributor Author

I don't think this AC has not been processed. "dans la story, l'utilisateur peut préciser la taille de l'élément et sa position"

I think the size part will be handled in the GET intergation. @clarani, what do you think?

@Uriel-Sautron
Copy link
Contributor

Thank you for this PR. Some differences with the sketch. Storybook: Screenshot From 2025-02-24 14-09-27
Sketch: Screenshot From 2025-02-24 14-08-11

  • The gov must take 100% of the width
  • The borders are different at the top and bottom of the split part

Thank you for your comment ! The aim of this PR is to integrate a component (any component) into the manchette. The “clean” integration of the gov will take place in a second PR, when this PR and that of the GET split will also be merged.

Okay, but we can't make a release that publishes this story, we still need to work on integration.

@theocrsb
Copy link
Contributor Author

Thank you for this PR. Some differences with the sketch. Storybook: Screenshot From 2025-02-24 14-09-27
Sketch: Screenshot From 2025-02-24 14-08-11

  • The gov must take 100% of the width
  • The borders are different at the top and bottom of the split part

Thank you for your comment ! The aim of this PR is to integrate a component (any component) into the manchette. The “clean” integration of the gov will take place in a second PR, when this PR and that of the GET split will also be merged.

Okay, but we can't make a release that publishes this story, we still need to work on integration.

Maybe we should publish an empty div story instead of the GOV? And insert the GOV into the PR with the GET?

@Akctarus
Copy link
Contributor

Done !

@Akctarus Akctarus force-pushed the t/update-manchette-props branch from c497378 to e796739 Compare February 26, 2025 10:36
@clarani
Copy link
Contributor

clarani commented Feb 27, 2025

I don't think this AC has not been processed. "dans la story, l'utilisateur peut préciser la taille de l'élément et sa position"

I think the size part will be handled in the GET intergation. @clarani, what do you think?

Yes it can be handled with the split of the GET 👍

Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

Code LGTM ! But you need to update also the ManchetteWithSpaceTimeChart stories. Can you check all the stories ? Thanks !

image

@theocrsb
Copy link
Contributor Author

Code LGTM ! But you need to update also the ManchetteWithSpaceTimeChart stories. Can you check all the stories ? Thanks !

image

I think it's good

@theocrsb theocrsb requested a review from clarani February 27, 2025 10:20
Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@theocrsb theocrsb force-pushed the t/update-manchette-props branch from 898efe2 to 9604df9 Compare February 27, 2025 11:07
Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

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

LGTM.

@theocrsb theocrsb enabled auto-merge February 27, 2025 11:14
@theocrsb theocrsb added this pull request to the merge queue Feb 27, 2025
Merged via the queue into dev with commit 88ff89d Feb 27, 2025
6 checks passed
@theocrsb theocrsb deleted the t/update-manchette-props branch February 27, 2025 11:19
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.

5 participants