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

SWED-2276 import & adapt for atlas icons #927

Merged
merged 25 commits into from
Feb 1, 2024

Conversation

goldenraphti
Copy link
Collaborator

@goldenraphti goldenraphti commented Dec 18, 2023

https://payexjira.atlassian.net/browse/SWED-2276
1st phase migrating icons from Google Material Icons to a combo Atlas icons & Swedbankpay custom icons

Material icons are still silently supported
but new icons are also supported

documentation have been complemented and updated (but not entirely rewritten, for lack of time, but it would be useful to do it at some point)

components are getting updated little by little.
They are all supporting new icons, but not all component code snippets have been updated

The DS website icons have not all been migrated yet

a 2nd iteration is needed to complete all of it.

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have updated the CHANGELOG document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Review instructions

Review instructions

Copy link

codecov bot commented Dec 18, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f2ff86a) 72.30% compared to head (60ae86f) 72.41%.
Report is 39 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #927      +/-   ##
===========================================
+ Coverage    72.30%   72.41%   +0.10%     
===========================================
  Files          212      212              
  Lines         4586     4604      +18     
  Branches      1306     1309       +3     
===========================================
+ Hits          3316     3334      +18     
  Misses        1126     1126              
  Partials       144      144              
Files Coverage Δ
src/App/AppHeader/index.js 57.14% <ø> (ø)
...onentsDocumentation/components/ActionList/index.js 90.90% <ø> (ø)
...onentsDocumentation/components/Alerts/constants.js 100.00% <ø> (ø)
...ComponentsDocumentation/components/Alerts/index.js 57.14% <ø> (ø)
...ComponentsDocumentation/components/Charts/index.js 93.33% <ø> (ø)
...pp/ComponentsDocumentation/components/Nav/index.js 63.63% <ø> (ø)
...ComponentsDocumentation/components/Select/index.js 53.84% <ø> (ø)
...ponentsDocumentation/components/Toast/constants.js 57.14% <ø> (ø)
.../App/GetStarted/get-started/ForDevelopers/index.js 86.11% <ø> (ø)
src/App/Home/constants.js 100.00% <ø> (ø)
... and 11 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c99df6...60ae86f. Read the comment docs.

Copy link

github-actions bot commented Dec 18, 2023

Size Change: +10.9 kB (+1%)

Total Size: 1.13 MB

Filename Size Change
dist/designguide/styles/payex.css 47.3 kB +2.7 kB (+6%) 🔍
dist/designguide/styles/swedbankpay.css 44.9 kB +2.59 kB (+6%) 🔍
dist/styles/payex.css 47.3 kB +2.7 kB (+6%) 🔍
dist/styles/swedbankpay.css 44.9 kB +2.59 kB (+6%) 🔍
ℹ️ View Unchanged
Filename Size Change
dist/designguide/scripts/dg-dashboard.js 72.9 kB 0 B
dist/designguide/scripts/dg.js 24.7 kB +14 B (0%)
dist/designguide/styles/documentation-payex.css 9.96 kB +43 B (0%)
dist/designguide/styles/documentation-swedbankpay.css 9.02 kB +41 B (0%)
dist/scripts/9438.js 71.8 kB 0 B
dist/scripts/dg-dashboard.js 72.9 kB 0 B
dist/scripts/dg.js 24.7 kB +14 B (0%)
dist/scripts/payex.js 322 kB +75 B (0%)
dist/scripts/swedbankpay.js 322 kB +79 B (0%)
dist/styles/documentation-payex.css 9.96 kB +43 B (0%)
dist/styles/documentation-swedbankpay.css 9.02 kB +41 B (0%)

compressed-size-action

@goldenraphti goldenraphti marked this pull request as ready for review January 5, 2024 23:47
Comment on lines 16 to 17
// TODO: is it actually needed ? why ? is it to make sure it's material icons normal and NOT material icons OUTLINED ?
// maybe we can remove it
Copy link
Collaborator

@stepandersen stepandersen Jan 29, 2024

Choose a reason for hiding this comment

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

I cannot see any reason for this rule, I suppose it is meant to do something with the close icon. but as it is implemented I cant see it fixing anything. I vote remove.

@@ -98,7 +96,7 @@
}
}

.material-icons {
i {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably a bit too generic since it is okay to pass html as content to the toast

stepandersen
stepandersen previously approved these changes Jan 29, 2024
@goldenraphti goldenraphti merged commit b7e3e63 into develop Feb 1, 2024
9 checks passed
@goldenraphti
Copy link
Collaborator Author

self-approved PR-merge after fixing comments from Stephan

@github-actions github-actions bot deleted the feature/SWED-2276-icon_library_atlas branch February 1, 2024 16:07
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.

2 participants