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-2249 create playbook doc #908

Merged
merged 39 commits into from
Oct 31, 2023

Conversation

goldenraphti
Copy link
Collaborator

@goldenraphti goldenraphti commented Oct 9, 2023

Don't get scared by the amount of files changed. Most of it (160 files) is image & video files, each with their own formats (images = .png + .avif , and videos are in .mp4 but in both the classic codec and as AV1 encoded, for performance reasons)

Also, since the sidebar got a new section, many tests files snapshots and screenshots had to be updated 😛 (3 screenshots)

Description

add the Playbook section to the DS documentation website
https://payexjira.atlassian.net/browse/SWED-2249

  • add Playbook section
  • remove Identity Overview page (replace the landing by the Identity/Accessibility

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

@goldenraphti goldenraphti changed the title Feature/swed 2249 create playbook doc SWED-2249 create playbook doc Oct 9, 2023
@github-actions
Copy link

github-actions bot commented Oct 10, 2023

Size Change: +3.59 kB (0%)

Total Size: 1.11 MB

Filename Size Change
dist/designguide/styles/documentation-payex.css 9.19 kB +620 B (+7%) 🔍
dist/designguide/styles/documentation-swedbankpay.css 8.28 kB +611 B (+8%) 🔍
dist/scripts/payex.js 322 kB +486 B (0%)
dist/scripts/swedbankpay.js 323 kB +479 B (0%)
dist/styles/documentation-payex.css 9.19 kB +620 B (+7%) 🔍
dist/styles/documentation-swedbankpay.css 8.28 kB +611 B (+8%) 🔍
ℹ️ View Unchanged
Filename Size Change
dist/designguide/scripts/dg-dashboard.js 72.9 kB 0 B
dist/designguide/scripts/dg.js 24.5 kB 0 B
dist/designguide/styles/payex.css 41.2 kB +41 B (0%)
dist/designguide/styles/swedbankpay.css 39.2 kB +42 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.5 kB 0 B
dist/styles/payex.css 41.2 kB +41 B (0%)
dist/styles/swedbankpay.css 39.2 kB +42 B (0%)

compressed-size-action

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reason of changes: we removed the Overview page from Identity section. Therfore each idenity page only has a link in the sidebar, there is no morelink in the (removed) Overview page. Now landing page for Identity is Accessibility

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reason of change:
Playbook is only for SwedbankPay.
So the homepage will still have only 4 quick-access cards. And whether it's SwedbankPay or PayEx one of them will either be Playbook Or Patterns

Comment on lines +38 to +42
{/* <source
type="image/avif"
media="(min-width: 576px)"
srcSet={`${basename}img/documentation/playbook/playbook-showroom-2.avif`}
/> */}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reasoning:
"Why keeping code commented-out?" You want to ask ?

  • Well, great question !! (I first added the images in avif too, to get better web-perf. But then designers asked me to yet increase the quality of the images. My issue is that I think the quality is good enough (3x everywhere) and I converted to avif in as high quality as possible and lossless. And with all the img/media content on this page the computers and phones are struggling to process and paint it all fluidly (well, they really struggle on a non 3000$ machine such as the crappy old laptop I am using, and several videos cannot even play). I do suspect this decision will change in the near future in an iteration for the necessity of actually having the page displaying its content fluidely even if it means slightly less quality). So here it is, ready to be un-commented at the snap of a finger 😉🤫

@@ -720,6 +720,183 @@
}
}

Copy link
Collaborator Author

@goldenraphti goldenraphti Oct 12, 2023

Choose a reason for hiding this comment

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

Note of the dev:
I am not 100% happy with all this .playbook-* styling. At all.
I think there is clearly room for simplification, reformatting it to have less "1 case rules".
So far we're a bit in a rush, so here it is.
Plus this is documentation website style only, and not even for the components, so it's (in my opinion) much less important to get it super nice and clean.
But I do want to go through it all again and improve & clean what can be 👍

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #908 (4611d09) into develop (bf3dece) will decrease coverage by 0.06%.
Report is 8 commits behind head on develop.
The diff coverage is 28.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #908      +/-   ##
===========================================
- Coverage    72.65%   72.60%   -0.06%     
===========================================
  Files          211      212       +1     
  Lines         4498     4508      +10     
  Branches      1257     1263       +6     
===========================================
+ Hits          3268     3273       +5     
- Misses        1093     1097       +4     
- Partials       137      138       +1     
Files Coverage Δ
...nentsDocumentation/components/Buttons/constants.js 100.00% <ø> (ø)
src/App/Home/constants.js 100.00% <ø> (ø)
src/App/Identity/index.js 100.00% <100.00%> (ø)
src/App/routes/identity.js 100.00% <ø> (ø)
src/App/routes/playbook.js 100.00% <100.00%> (ø)
src/App/routes/all.js 87.50% <75.00%> (-12.50%) ⬇️
src/App/index.js 39.47% <11.11%> (+0.58%) ⬆️
src/App/Home/index.js 17.39% <10.00%> (+1.60%) ⬆️

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 7384728...4611d09. Read the comment docs.

MariusSjo
MariusSjo previously approved these changes Oct 17, 2023
Copy link

@MariusSjo MariusSjo left a comment

Choose a reason for hiding this comment

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

I think it looks good!

It is interesting to see the "conflict" on the user's loading time vs quality of the visualisations on the page.

I think it is important to make our product available. However, if the users are developers with good internet connection and fast computers we might not have to have this discussion as prio 1. it would be nice with some guidelines on what to prioritise.

Nice job ✅

@goldenraphti
Copy link
Collaborator Author

goldenraphti commented Oct 31, 2023

bypassing approval since it was approved by Marius, and only had to modify the release-notes markdown for merge-conflict 😉

@goldenraphti goldenraphti merged commit e7de957 into develop Oct 31, 2023
8 checks passed
@github-actions github-actions bot deleted the feature/SWED-2249-create_playbook_doc branch October 31, 2023 14:55
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