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

ConditionsModal: Fix modal component structure and add missing translations #14782

Merged
merged 3 commits into from Nov 4, 2022

Conversation

gu-stav
Copy link
Contributor

@gu-stav gu-stav commented Nov 4, 2022

What does it do?

Fixes the conditions modal:

  • Adds a missing translation to the header
  • Removes the duplicated headline
  • Fixes the modal padding
Before After
Screenshot 2022-11-04 at 11 19 49 Screenshot 2022-11-04 at 11 19 33

Why is it needed?

While looking into #14763 @maevalienard and I realized neither the padding of the ModalBody matches, nor has the header the proper translation.

How to test it?

  1. Start Strapi in EE mode
  2. Navigate to http://localhost:4000/admin/settings/roles/2
  3. Open settings for one content-type

Related issue(s)/PR(s)

@gu-stav gu-stav added source: core:admin Source is core/admin package pr: fix This PR is fixing a bug labels Nov 4, 2022
@gu-stav gu-stav added this to the 4.5.0 milestone Nov 4, 2022
@codecov
Copy link

codecov bot commented Nov 4, 2022

Codecov Report

Base: 58.72% // Head: 63.19% // Increases project coverage by +4.47% 🎉

Coverage data is based on head (71a7c1b) compared to base (933f646).
Patch coverage: 16.66% of modified lines in pull request are covered.

❗ Current head 71a7c1b differs from pull request most recent head 006f1e3. Consider uploading reports for the commit 006f1e3 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14782      +/-   ##
==========================================
+ Coverage   58.72%   63.19%   +4.47%     
==========================================
  Files        1339     1050     -289     
  Lines       32490    22360   -10130     
  Branches     6143     3909    -2234     
==========================================
- Hits        19079    14131    -4948     
+ Misses      11523     7256    -4267     
+ Partials     1888      973     -915     
Flag Coverage Δ
front 63.19% <16.66%> (-0.01%) ⬇️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...e/components/ContentTypeCollapse/Collapse/index.js 41.89% <ø> (ø)
...Roles/EditPage/components/ConditionsModal/index.js 34.00% <16.66%> (-3.74%) ⬇️
...es/core/admin/utils/create-plugins-exclude-path.js
packages/core/database/lib/query/helpers/join.js
packages/core/database/lib/fields/field.js
packages/core/utils/lib/sanitize/sanitizers.js
packages/core/admin/server/utils/index.js
packages/core/utils/lib/parse-multipart.js
...ges/core/admin/server/domain/condition/provider.js
packages/core/database/lib/fields/time.js
... and 281 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link

@maevalienard maevalienard left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you :)

@gu-stav gu-stav merged commit a11193d into main Nov 4, 2022
@gu-stav gu-stav deleted the fix/settings-conditions branch November 4, 2022 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: core:admin Source is core/admin package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants