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

Introduce optional uniform appearance of Modal #DS-1091 #1192

Merged

Conversation

adamkudrna
Copy link
Contributor

@adamkudrna adamkudrna commented Dec 19, 2023

Description

Snímek obrazovky 2023-12-19 v 14 39 39

Add spirit-feature-modal-enable-uniform-dialog feature class to enable uniform appearance of Modal across all breakpoints.

Current mobile design is then accessible using the Modal--dockOnMobile modifier class.

Additional context

We need to make sure it works in all scenarios:

  1. Current behavior must remain intact.
  2. New appearance can be turned on via CSS class,
  3. … or via Sass feature flag.

Issue reference

https://jira.lmc.cz/browse/DS-1091

@adamkudrna adamkudrna self-assigned this Dec 19, 2023
Copy link

netlify bot commented Dec 19, 2023

Deploy Preview for spirit-design-system-demo ready!

Name Link
🔨 Latest commit b4fccc4
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-demo/deploys/6596cf500684dd0008ebf277
😎 Deploy Preview https://deploy-preview-1192--spirit-design-system-demo.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.

Copy link

netlify bot commented Dec 19, 2023

Deploy Preview for spirit-design-system-react canceled.

Name Link
🔨 Latest commit b4fccc4
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-react/deploys/6596cf5064a188000833a58f

Copy link

netlify bot commented Dec 19, 2023

Deploy Preview for spirit-design-system-validations canceled.

Name Link
🔨 Latest commit b4fccc4
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-validations/deploys/6596cf5006b6980008266849

@github-actions github-actions bot added the feature New feature or request label Dec 19, 2023
Copy link

netlify bot commented Dec 19, 2023

Deploy Preview for spirit-design-system-storybook canceled.

Name Link
🔨 Latest commit b4fccc4
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-storybook/deploys/6596cf50363de60007f017b6

Copy link
Contributor

Coverage Status

coverage: 70.462%. remained the same
when pulling b6d9429 on feature/web-modal-uniform-design-on-all-breakpoints
into 8acf2e7 on main.

Copy link
Member

@crishpeen crishpeen left a comment

Choose a reason for hiding this comment

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

LGTM, I have just a few small thoughts.

Copy link
Contributor

@pavelklibani pavelklibani left a comment

Choose a reason for hiding this comment

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

Just two questions, otherwise LGTM

packages/web/src/scss/components/Modal/README.md Outdated Show resolved Hide resolved
packages/web/src/scss/components/Modal/index.html Outdated Show resolved Hide resolved
@adamkudrna adamkudrna force-pushed the feature/web-modal-uniform-design-on-all-breakpoints branch from b6d9429 to a8b3577 Compare January 2, 2024 17:57
@coveralls
Copy link

coveralls commented Jan 2, 2024

Coverage Status

coverage: 76.799% (-19.7%) from 96.491%
when pulling b4fccc4 on feature/web-modal-uniform-design-on-all-breakpoints
into 3a6f9f0 on main.

Copy link
Collaborator

@literat literat left a comment

Choose a reason for hiding this comment

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

I have written down a few comments, otherwise the appearance is looking good. Nice work 👍

@adamkudrna adamkudrna force-pushed the feature/web-modal-uniform-design-on-all-breakpoints branch from a8b3577 to 51ef849 Compare January 4, 2024 15:15
Add `spirit-feature-modal-enable-uniform-dialog` feature class to enable
uniform appearance of `Modal` across all breakpoints.

Current mobile design is then accessible using the `Modal--dockOnMobile` modifier class.
…nsumption

Firefox warns about potential high memory consumption when `will-change`
is used on big areas. Therefore it seems better to remove it in this case to
free up the memory for smaller tweaks in Spirit and also for other performance
optimisations our users might want to make.

https://dev.opera.com/articles/css-will-change-property/
@adamkudrna adamkudrna force-pushed the feature/web-modal-uniform-design-on-all-breakpoints branch from 51ef849 to b4fccc4 Compare January 4, 2024 15:31
@adamkudrna adamkudrna merged commit cbe878b into main Jan 4, 2024
28 checks passed
@adamkudrna adamkudrna deleted the feature/web-modal-uniform-design-on-all-breakpoints branch January 4, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants