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

IBX-2399: Introduce reusable twig modal component #335

Merged
merged 6 commits into from
Mar 7, 2022

Conversation

@tischsoic tischsoic self-assigned this Feb 21, 2022
@@ -296,7 +296,7 @@ $modal-header-border-color: $ibexa-color-light-500;
$modal-title-font-size: 28px;
$modal-content-border-width: 0;
$modal-content-border-radius: $ibexa-border-radius;
$modal-inner-padding: 24px 0;
$modal-inner-padding: calculateRem(24px);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has to be single value in rem because it is used in such expression in bootstrap scss internals:
padding: $modal-inner-padding - $modal-footer-margin-between * .5;
ref. https://github.com/ibexa/admin-ui-assets/blob/792cd47c43ab5cc05609e6a45242ef87934d5afb/src/bundle/Resources/public/vendors/bootstrap/scss/_modal.scss#L131
otherwise it produces:
Screenshot 2022-02-21 at 21 09 53

@@ -3,5 +3,5 @@
@function calculateRem($size) {
$remSize: div($size, $base-font-size);

@return #{$remSize}rem;
@return $remSize + 0rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous version was actually not converting values to rem measurement in SCSS. Change required for $modal-inner-padding;

@tischsoic tischsoic force-pushed the IBX-2399-introduce-modal-twig-component branch from 80b8b9f to fde381a Compare February 24, 2022 07:24
@tischsoic tischsoic changed the base branch from main to IBX-2403-fix-calculateRem February 24, 2022 07:25

{% set no_header = no_header|default(false) %}

{% set size = size|default(false) %}
Copy link
Contributor

Choose a reason for hiding this comment

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

as discussed in private, no mixed type.

Base automatically changed from IBX-2403-fix-calculateRem to main February 25, 2022 13:34
@tischsoic tischsoic force-pushed the IBX-2399-introduce-modal-twig-component branch from fde381a to 0855b1e Compare February 28, 2022 14:28
@tischsoic tischsoic marked this pull request as ready for review February 28, 2022 14:29
@tischsoic tischsoic force-pushed the IBX-2399-introduce-modal-twig-component branch from ef2d1fd to a457541 Compare March 4, 2022 08:43
@micszo micszo self-assigned this Mar 4, 2022
Copy link
Contributor

@micszo micszo left a comment

Choose a reason for hiding this comment

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

QA Approved on Ibexa Commerce 4.0.3-dev with 11 branches.

@dew326 dew326 merged commit 89e6871 into main Mar 7, 2022
@dew326 dew326 deleted the IBX-2399-introduce-modal-twig-component branch March 7, 2022 08:51
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