Modal: fix margin between text and buttons

[MewenLeHo]

Related issues

Closes #1331

Description

Adding spacing utilities.

Motivation & Context

Respect DSM for modals: https://system.design.orange.com/0c1af118d/p/16d9f3-modals/b/774d3d/i/66611040

Types of change

• New feature (non-breaking change which adds functionality)

Live previews

• https://deploy-preview-1658--boosted.netlify.app/docs/5.3/components/modal/#examples

Checklist

• I have read the contributing guidelines
• My code follows the code style of the project (using npm run lint)
• My change introduces changes to the documentation
• My change introduces changes to the migration guide
• I have updated the documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed

[netlify]

✅ Deploy Preview for boosted ready!

Name	Link
🔨 Latest commit	be6c3cf
🔍 Latest deploy log	https://app.netlify.com/sites/boosted/deploys/637e1c1c4e9e9b000825e499
😎 Deploy Preview	https://deploy-preview-1658--boosted.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[netlify]

✅ Deploy Preview for boosted ready!

Name	Link
🔨 Latest commit	20ee710
🔍 Latest deploy log	https://app.netlify.com/sites/boosted/deploys/64084de0e63d0a0008868aff
😎 Deploy Preview	https://deploy-preview-1658--boosted.netlify.app/docs/5.3/migration
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[louismaximepiton]

I think such modifications should definitely be applied on all modals in the doc.

That said, 3 points to discuss about:

• actually wondering if those spacings should be in the modal classes themselves since it will be more transparent to users
• Shall the spacings be pixel perfect ?
• Shall we change the spacings between .modal-header and .modal-body to be brand compliant ? (20px, sm+:25px)

[louismaximepiton]

LGTM 🚀

[julien-deramond]

LGTM! Last step is the design review and then 🚀

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[julien-deramond]

It's difficult to read the history with the force pushes. Is there anything new in this PR in the last commits I need to check since my last review?

[MewenLeHo]

@Franco-Riccitelli: thanks a lot for your helpful comment. As I said I opened a distinct issue for the redesign of modals.

The current PR will only tackle the probleme described in the related issue #1331: the distance between .modal-body and the buttons contained in .modal-footer

According to your screenshot, it must be 35px in desktop and 30px in mobile.

In this PR we currently have:

• in desktop: 10px coming from .modal-body padding and 15px from its margin + 10px coming from .modal-footer padding =>35px
• in mobile: 10px coming from .modal-body padding and 10px from its margin + 10px coming from .modal-footer padding =>30px

So I think we are good. @louismaximepiton, you made the review, is it still ok for you?

Last remaining point according to your screenshots @Franco-Riccitelli, modals with scrolling are an edge case with a spacing of only 20px in desktop AND mobile.

Currently we have:

• in desktop: 10px coming from .modal-body margin + 10px coming from .modal-footer padding =>20px
• in mobile: 10px coming from .modal-body margin + 10px coming from .modal-footer padding =>20px

So no change needed.

Is it ok for both of you?

[MewenLeHo]

@Franco-Riccitelli: is the 'ok design' label ok for you?

[julien-deramond]

@MewenLeHo What if there's no .modal-footer? I didn't think about it before but IMO there should be a rule not to apply the padding in this case. Or maybe we consider that users should use .mb-0 in this very specific use case?

[louismaximepiton]

There's an issue with the spacing between the line and the buttons inside the scrollable content one at the bottom. Maybe part of the last commit ?

[MewenLeHo]

There's an issue with the spacing between the line and the buttons inside the scrollable content one at the bottom. Maybe part of the last commit ?

It should be better after the fix. But I had to add a specific variable for the scrollable modals and I am still not sure if I like it or not... 🤔

[louismaximepiton]

Few things to tackle

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication