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

feat: add positioning options to FwbModal #293

Merged
merged 5 commits into from
Aug 6, 2024

Conversation

woodrunsdeep
Copy link
Contributor

@woodrunsdeep woodrunsdeep commented Jun 22, 2024

Currently FwbModal doesn't have position prop to control modal's placement on the screen. This PR adds this feature taking into consideration possible usage in RTL (Right-to-Left) mode.

  1. Modal's parent element is display: block and the position prop sets corresponding classes responsible for modal's alignment. Default position is center.
  2. There's a small fix of close button alignment in RTL mode - usage of logical property ms-auto instead of ml-auto.
  3. As was mentioned here on current implementation click outside (on top/under a modal specifically) doesn't close a modal. Fixed with this PR as well.

Closes: #279 #235

Copy link

netlify bot commented Jun 22, 2024

Deploy Preview for sensational-seahorse-8635f8 ready!

Name Link
🔨 Latest commit ef68d30
🔍 Latest deploy log https://app.netlify.com/sites/sensational-seahorse-8635f8/deploys/6676dd670bbda100080b21f6
😎 Deploy Preview https://deploy-preview-293--sensational-seahorse-8635f8.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@woodrunsdeep
Copy link
Contributor Author

Although maybe I should have used Flowbite's implementation to be in line with the core lib:

  • Use flex on a parent and set alignment classes on a parent as well
  • Name prop placement

But I think we should keep logical prop value names (top-start, bottom-end, etc.) instead of Flowbite's top-left, bottom-right cause in RTL mode top-left will place a model to the top right corner.

@Sqrcz If my assumptions are correct please let me know and I'll do the rest.

@fdeitelhoff
Copy link

Thanks for the fix.

Any updates on a publish timeline? This would be a nice feature for a project right now. :)
Or can I update the files manually, until a new version is ready?

Thanks!

@woodrunsdeep
Copy link
Contributor Author

@Sqrcz @cogor

Hi. It's been a while since this PR was created. I would appreciate your feedback on it.

Thanks

@cogor
Copy link
Collaborator

cogor commented Jul 16, 2024

@Sqrcz @cogor

Hi. It's been a while since this PR was created. I would appreciate your feedback on it.

Thanks

Sorry, unfortunately I don't have much time for OS, but I'll do a review your PR ASAP

Copy link
Collaborator

@Sqrcz Sqrcz left a comment

Choose a reason for hiding this comment

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

I've tested it and it works as it should. 🎉

I like this naming better for the reasons you've mentioned... 💯

Solid work, thank you!

@Sqrcz Sqrcz merged commit bce8477 into themesberg:main Aug 6, 2024
4 checks passed
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.

FwbModal vertical position
4 participants