Mwpw 142267: What's included style fixes #2674
Conversation
Axelcureno
added
run-nala
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2674 +/- ##
==========================================
- Coverage 95.90% 95.90% -0.01%
==========================================
Files 173 173
Lines 45842 45842
==========================================
- Hits 43967 43964 -3
- Misses 1875 1878 +3 ☔ View full report in Codecov by Sentry. |
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
| margin-bottom: 10px; | ||
| gap: 5px; | ||
| margin-bottom: 5px; | ||
| margin-right: 10px; |
There was a problem hiding this comment.
Is this reflected for RTL?
There was a problem hiding this comment.
Layout is the same for RTL geos in this case.
There was a problem hiding this comment.
That sounds a bit odd, why is the layout not mirrored? Additionally, could you provide some testing details? I did visit the preview URL, but got a completely different modal than what was added here. I also switched to RTL to see how the modal reacts and I see the label covers the icon:
There was a problem hiding this comment.
To test the experience you have to click on "See all included apps" on the CC All Apps card:
The whole TwP experience is not yet build E2E though, but we decided to merge this WIP since our focus shifted to M@S v2.
There was a problem hiding this comment.
Ok, I see, thanks for sharing! Not having RTL is still a bit odd, maybe you can open a story to address it in the future.
| margin-bottom: 10px; | ||
| gap: 5px; | ||
| margin-bottom: 5px; | ||
| margin-right: 10px; | ||
| align-items: flex-end; |
There was a problem hiding this comment.
Wouldn't center be better suited here?
| margin-bottom: 10px; | ||
| gap: 5px; | ||
| margin-bottom: 5px; | ||
| margin-right: 10px; |
There was a problem hiding this comment.
Ok, I see, thanks for sharing! Not having RTL is still a bit odd, maybe you can open a story to address it in the future.
|
This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label. |
|
@Axelcureno When possible plz update from latest stage and also address psi failure thanks |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
Approving this since this fixes few discrepancy , although there are yet development yet to happen on this piece and also other issues needs to be addressed which are marked in TBD: https://jira.corp.adobe.com/browse/MWPW-154370
Roycethan
added
verified
|
Skipped merging 2674: Mwpw 142267: What's included style fixes due to failing checks |
This PR is needed for #2554
Fixes styles for what's included TwP screen.
Preiew URL: https://mwpw-142267--milo--adobecom.hlx.page/drafts/axel/block-samples/twp/illustrator#twpmodal
Test URLs:
Before: https://main--mas--adobecom.hlx.live/
After: https://mwpw-142267--mas--adobecom.hlx.live/