SWED-2275 cards tweaks code-review

SwedbankPay · Mar 20, 2024 · 8e5b0a6 · 8e5b0a6
1 parent 4cc316e
commit 8e5b0a6
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 10 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -6,7 +6,10 @@
 
 - Cards
   - new Cards UI. To migrate requires code changes from DG consuming devs. BUT silent support for "old" card syntax. Please check the documentation for more details.
+  - Silent support of the old UI until for the codebases who have not yet migrated their cards will be supported until the next major version release.
   - the new UI requires the introduction of new classes (.cards-cta, span.arrow, ...). Unless one of these new classes (`.cards-cta`) is present, then the old UI will be used, by checking conditionally `cards:has(.cards-cta)`. If your project does NOT support the :has() selector yet, but requires fallback support for the old UI, then add `.legacy` class to the `.cards` element.
+  - deprecated cards variants (.primary, .secondary, .tertiary)
+  - cards title text need to have the `.h4` class
 
 ## Technical changes
 

diff --git a/src/App/ComponentsDocumentation/components/Card/constants.js b/src/App/ComponentsDocumentation/components/Card/constants.js
@@ -225,12 +225,12 @@ export const overviewCards = {
 								},
 							},
 							{
-								name: "With image - 1/1 ratio",
+								name: "With image",
 								value: {
 									imgSrc:
 										"https://design.swedbankpay.com/v/10.10.1/img/documentation/imagery/women-on-bus.svg",
 									icon: null,
-									imgRatio: "ratio-1-1",
+									imgRatio: null,
 								},
 							},
 						],

diff --git a/src/App/components/Cards/index.js b/src/App/components/Cards/index.js
@@ -27,7 +27,6 @@ const Cards = ({
 	imgRatio,
 }) => (
 	<>
-		{/* TODO: set conditional <a> || <button> */}
 		<OuterTagElement isButton={isButton} isWide={isWide}>
 			{imgSrc && (
 				<>
@@ -78,8 +77,11 @@ Cards.propTypes = {
 	iconClasses: PropTypes.string,
 	textContent: PropTypes.string,
 	children: PropTypes.object,
-	wide: PropTypes.bool,
 	imgRatio: PropTypes.string,
+	hasTextContent: PropTypes.bool,
+	isWide: PropTypes.bool,
+	isButton: PropTypes.bool,
+	hasCTAText: PropTypes.bool,
 };
 
 export default Cards;
diff --git a/src/App/components/Cards/index.test.js b/src/App/components/Cards/index.test.js
@@ -5,8 +5,6 @@ import "@testing-library/jest-dom";
 
 import Cards from "./index";
 
-// TODO: go through each change made to evaluate if necessary (both internal changes to the React component && especially externally to classes made etc), and if necessary then document it
-
 describe("Component: Cards -", () => {
 	it("is defined", () => {
 		expect(Cards).toBeDefined();

diff --git a/src/less/components/card.less b/src/less/components/card.less
@@ -1,7 +1,5 @@
 @import "../global.less";
 
-// TODO: sera que el card title TIENE que llevar la clase .h4 ? or ponemos un title mas abstracto?
-
 .cards:has(.cards-cta) {
 	--border-radius: 16px;
 	--cards-light-gray: var(--base-5);
@@ -264,14 +262,15 @@
 				aspect-ratio: 1;
 			}
 
-			& img:first-child {
+			& > img:first-child {
 				max-width: 100%;
 				object-fit: cover;
 				width: auto;
 				height: 100%;
 				max-height: 100%;
 				position: absolute;
 				left: 0;
+				aspect-ratio: 1;
 			}
 
 			&:not(:has(.cards-content)) {
@@ -303,7 +302,6 @@
 		grid-area: text-content;
 	}
 
-	// so far NOT set as mandatory parent of i (arrow at bottom of card) so don't break existing cards
 	.cards-cta {
 		grid-area: cta;
 		align-self: end;