Fixed SignInUp Modal misalignment for devices smaller than 400px width

[ehconitin]

Hi @Bonapara,
Issue #6385

I encountered an issue with the Modal component where its width was fixed at 400px. While the container housing the Modal adjusted its size based on the screen width, the Modal itself remained at 400px regardless of the screen size.

I have implemented a change to address this problem. Could you please review the changes and let me know your thoughts?

Thank you!

2024-07-24.14-00-05.mp4

[greptile-apps]

PR Summary

The pull request addresses the misalignment issue of the SignInUp Modal on devices with a width smaller than 400px by removing the fixed width constraint.

• Removed fixed width: packages/twenty-front/src/modules/auth/components/Modal.tsx - Eliminated the hardcoded 400px width from the StyledContent styled component to enhance responsiveness.
• Improved alignment: The modal now dynamically adjusts to fit smaller screen sizes, ensuring better user experience on devices under 400px width.

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings}

[ehconitin]

@Bonapara
Addressed issue #5740

for devices width viewport less than 768px(mobile) will have onboarding screen fullscreened.
Btw I noticed one more bug, pricing screen does not go into dark mode.

2024-07-25.16-31-14.mp4

2024-07-25.16-37-18.mp4

[Bonapara]

Hi @ehconitin, thanks so much for this PR. Do you think you can address the issue of pricing not switching to dark mode in the same PR?

[ehconitin]

@Bonapara
I did look into it a bit.
I think that needs further digging.
I will get back to you if I figure it out.

[ehconitin]

@Bonapara

I reviewed the issue again, and it appears the problem isn't with the pricing screen but with the signup page itself. When the mode is selected as light, the pricing page renders with the correct theme colors, indicating that the issue lies with the signup page.

Its the same with non mobile screens

Please refer to the video for more details.

2024-07-25.21-51-31.mp4

[ehconitin]

@Bonapara
I made changes to ensure consistent theme management just like we discussed.
I would like to know if this a good approach, if yes I will cleanup the code a bit for merge.

[ehconitin]

I had a look at PR#5758

Creating variants should be easy.
Ill do that .
Right now the deletion modal on mobile screen goes fullscreen and not aligned.
Will fix it.

[ehconitin]

Commited a temporay fix for modals.
This works.
But I think Modal overhaul is required.
It is being used in many places with atleast 3 different wrappers I could find in short time.

[ehconitin]

@FelixMalfait @Bonapara

I have created a new EnhancedModalLayout component that integrates the closable functionality, thereby consolidating the basic layout and closing properties into a single component. This change eliminates the need for two separate components, ModalLayout and Modal, for these functionalities.

Additionally, I introduced modalVariants as requested in this comment.

Could you please confirm if the spreadsheet-import modal needs to be hotkey-scoped? For now, I have added it. I have reviewed all modals to ensure the behavior is consistent, but please let me know if I missed anything.

Note: I have not deleted the old Modal component yet but it is redundant after this change. I will clean up the code after receiving your feedback.

[Bonapara]

@lucasbordeau, maybe you have some thoughts too!

[FelixMalfait]

Will be a nice improvement! I think I didn't fully get the intention/strategy behind that new component, hopefully we can clarify and merge this. Thanks (again and again!) for your work

[ehconitin]

@FelixMalfait

This is how the code would look after changes.
removed original ModalLayout and Modal, renamed EnhancedModalLayout to Modal
Could you please review once more :)
Please let me know if I missed something.

[ehconitin]

@FelixMalfait
I had a question regarding this import modal.
This was not initially hotscoped probably because accidental click outside while importing will close the modal
This comment is more of a question.
I forgot to raise this earlier.

[FelixMalfait]

Yes, the behavior seems acceptable like that to me

[ehconitin]

wait, so should be hotscoped or not?
I am sorry but Just want to confirm, cuz accidental touch might lead to loss of all that import steps user might be doing.
Presently it is hotscoped which it initially was not.

[github-actions]

Thanks @ehconitin for your contribution!
This marks your 0th PR on the repo. You're top 0% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter