jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/392077 )

Change subject: Chore: extract Thumbnail to separate component
......................................................................


Chore: extract Thumbnail to separate component

Thumbnail is currently used by the summary endpoint and soon to be used
by the image File pages. Extract the component to a distinct file so it
can be reused.

Bug: T179829
Change-Id: I65cf8246f4285828f1b0070c1fb8c0d85266d962
---
M package.json
M src/common/components/page-summary/page-summary.css
M src/common/components/page-summary/page-summary.tsx
A src/common/components/thumbnail/thumbnail.css
A src/common/components/thumbnail/thumbnail.tsx
5 files changed, 93 insertions(+), 76 deletions(-)

Approvals:
  Jhernandez: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/package.json b/package.json
index 1d5d82c..72e1e48 100644
--- a/package.json
+++ b/package.json
@@ -127,7 +127,7 @@
     },
     {
       "path": "dist/public/pages/summary.*.js",
-      "maxSize": "2.9KB"
+      "maxSize": "3KB"
     },
     {
       "path": "dist/public/pages/wiki.*.js",
diff --git a/src/common/components/page-summary/page-summary.css 
b/src/common/components/page-summary/page-summary.css
index 5f77601..a1a3894 100644
--- a/src/common/components/page-summary/page-summary.css
+++ b/src/common/components/page-summary/page-summary.css
@@ -15,54 +15,5 @@
   visibility: hidden;
 }
 
-.PageSummary-thumbnail {
-  background: var(--wmui-color-base0);
-
-  /* Center the image within the container. */
-  text-align: center;
-
-  /* Set the container to occupy the full-width. */
-  display: inline-block;
-  width: 100%;
-
-  /* Since the image is below the extract, set a top margin. */
-  margin-top: var(--space);
-}
-/* TODO: use CSS variable. */
-@media (min-width: 1024px) {
-  .PageSummary-thumbnail {
-    /* Wrap the container to the width of the image and left-align it. Floating
-       takes precedence over display. */
-    width: unset;
-    float: left;
-
-    /* Since the image is both side-by-side and below the extract, set a right
-       margin too. Since the side-by-side text may wrap around, set a bottom
-       margin. */
-    margin-right: var(--space);
-    margin-bottom: var(--space);
-  }
-}
-.PageSummary-thumbnail-image {
-  /* Don't pad the linked image with text descender space. */
-  vertical-align: top;
-}
-.PageSummary-thumbnail-image-landscape {
-  /* Stretch the image to occupy the available width and proprotionally scale
-     the height. */
-  width: 100%;
-  height: auto;
-}
-/* TODO: use CSS variable. */
-@media (min-width: 1024px) {
-  .PageSummary-thumbnail-image-landscape {
-    /* The screen is too large to maximize the image. Use the image's native
-       width and height. */
-    width: unset;
-    height: unset;
-  }
-}
-.PageSummary-thumbnail-image-portrait {}
-
 .PageSummary-extract-lead {}
 .PageSummary-extract-body {}
diff --git a/src/common/components/page-summary/page-summary.tsx 
b/src/common/components/page-summary/page-summary.tsx
index cd13a9a..93839c5 100644
--- a/src/common/components/page-summary/page-summary.tsx
+++ b/src/common/components/page-summary/page-summary.tsx
@@ -2,6 +2,7 @@
 import "./page-summary.css";
 import { PageSummary as PageSummaryModel } from "../../models/page/summary";
 import Content from "../content/content";
+import { Thumbnail } from "../thumbnail/thumbnail";
 
 export interface Props {
   summary: PageSummaryModel;
@@ -15,36 +16,17 @@
         class="PageSummary-extract-lead"
         dangerouslySetInnerHTML={{ __html: lead }}
       />
-      <Thumbnail summary={summary} />
+      {summary.thumbnail && (
+        <Thumbnail
+          class="PageSummary-thumbnail"
+          image={summary.thumbnail}
+          url={summary.image && summary.image.url}
+        />
+      )}
       <div
         class="PageSummary-extract-body"
         dangerouslySetInnerHTML={{ __html: body.join("") }}
       />
     </div>
-  );
-};
-
-const Thumbnail = ({ summary }: Props) => {
-  if (!summary.thumbnail || !summary.image) {
-    return null;
-  }
-
-  const landscape = summary.image.landscape;
-  const imageOrientationClass = `PageSummary-thumbnail-image-${landscape
-    ? "landscape"
-    : "portrait"}`;
-  return (
-    // todo: replace anchor with Link.
-    <span class="PageSummary-thumbnail">
-      <a href={summary.image.url}>
-        <img
-          key={summary.thumbnail.url}
-          class={`PageSummary-thumbnail-image ${imageOrientationClass}`}
-          src={summary.thumbnail.url}
-          width={summary.thumbnail.width}
-          height={summary.thumbnail.height}
-        />
-      </a>
-    </span>
   );
 };
diff --git a/src/common/components/thumbnail/thumbnail.css 
b/src/common/components/thumbnail/thumbnail.css
new file mode 100644
index 0000000..e872aac
--- /dev/null
+++ b/src/common/components/thumbnail/thumbnail.css
@@ -0,0 +1,54 @@
+
+.Thumbnail {
+  background: var(--wmui-color-base0);
+
+  /* Center the image within the container. */
+  text-align: center;
+
+  /* Set the container to occupy the full-width. */
+  display: inline-block;
+  width: 100%;
+
+  /* Since the image is below the extract, set a top margin. */
+  margin-top: var(--space);
+}
+
+/* TODO: use CSS variable. */
+@media (min-width: 1024px) {
+  .Thumbnail {
+    /* Wrap the container to the width of the image and left-align it. Floating
+       takes precedence over display. */
+    width: unset;
+    float: left;
+
+    /* Since the image is both side-by-side and below the extract, set a right
+       margin too. Since the side-by-side text may wrap around, set a bottom
+       margin. */
+    margin-right: var(--space);
+    margin-bottom: var(--space);
+  }
+}
+
+.Thumbnail-image {
+  /* Don't pad the linked image with text descender space. */
+  vertical-align: top;
+}
+
+.Thumbnail-image-landscape {
+  /* Stretch the image to occupy the available width and proprotionally scale
+     the height. */
+  width: 100%;
+  height: auto;
+}
+
+/* TODO: use CSS variable. */
+@media (min-width: 1024px) {
+  .Thumbnail-image-landscape {
+    /* The screen is too large to maximize the image. Use the image's native
+       width and height. */
+    width: unset;
+    height: unset;
+  }
+}
+
+.Thumbnail-image-portrait {}
diff --git a/src/common/components/thumbnail/thumbnail.tsx 
b/src/common/components/thumbnail/thumbnail.tsx
new file mode 100644
index 0000000..64dd7a2
--- /dev/null
+++ b/src/common/components/thumbnail/thumbnail.tsx
@@ -0,0 +1,30 @@
+import { h } from "preact";
+import { ClassProps, classOf } from "../preact-utils";
+import Link from "../link";
+import { PageImage } from "../../models/page/image";
+import "./thumbnail.css";
+
+export interface Props {
+  image: PageImage;
+  url?: string;
+}
+
+export function Thumbnail({ image, url, ...props }: Props & ClassProps) {
+  const imageOrientationClass = `Thumbnail-image-${image.landscape
+    ? "landscape"
+    : "portrait"}`;
+  const img = (
+    <img
+      key={image.url}
+      class={`Thumbnail-image ${imageOrientationClass}`}
+      src={image.url}
+      width={image.width}
+      height={image.height}
+    />
+  );
+  return (
+    <span class={classOf("Thumbnail", props.class)}>
+      {url ? <Link href={url}>{img}</Link> : img}
+    </span>
+  );
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/392077
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I65cf8246f4285828f1b0070c1fb8c0d85266d962
Gerrit-PatchSet: 3
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: Jhernandez <jhernan...@wikimedia.org>
Gerrit-Reviewer: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: Sniedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to