[OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const…

…' keyword and CSS modifier '--last' for one class only (opensearch-project#1144) * refactoring changing 'let' variables with 'const' Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * refactoring changing 'let' variables with 'const' Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBreadcrumb class, keeping it only in .ouiBreadcrumbWrapper class Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBreadcrumb class, keeping it only in .ouiBreadcrumbWrapper class Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * updating CHANGELOG.md file Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * fixing calculation of calculatedMax, breadcrumbs.tsx, line 310 Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * updating breadcrumbs JSX according suggestion from maintainer for better readability of code Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> --------- Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com>
BigSamu · Jan 11, 2024 · 064a02c · 064a02c
1 parent 6d5c3b9
commit 064a02c
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -50,6 +50,7 @@
 - Clean up `react-datepicker` package to remove unnecessary directories and files ([#1067](https://github.com/opensearch-project/oui/pull/1067))
 - Bump `@types/react` and `csstype` ([#1105](https://github.com/opensearch-project/oui/pull/1105))
 - Add `scripts` folder to lint-es script ([#1143](https://github.com/opensearch-project/oui/pull/1143))
+- Clean up code OUI Breadcrumb component from previous updates ([#1144](https://github.com/opensearch-project/oui/pull/1144))
 - Update deprecated Babel plugins ([#1155](https://github.com/opensearch-project/oui/pull/1155))
 - Move @seanneumann to emeritus maintainer ([#1188](https://github.com/opensearch-project/oui/pull/1188))
 

diff --git a/src/components/breadcrumbs/_breadcrumbs.scss b/src/components/breadcrumbs/_breadcrumbs.scss
@@ -25,18 +25,15 @@
 
 .ouiBreadcrumb {
   display: inline-block;
+  // TODO: remove important: https://github.com/opensearch-project/oui/issues/376
+  color: $ouiBreadcrumbInactiveTextColor !important; // sass-lint:disable-line no-important
 
-  &:not(.ouiBreadcrumb--last) {
-    // TODO: remove important: https://github.com/opensearch-project/oui/issues/376
-    color: $ouiBreadcrumbInactiveTextColor !important; // sass-lint:disable-line no-important
-
-    &:hover {
-      color: $ouiBreadCrumbHoverColor !important; // sass-lint:disable-line no-important
-    }
+  &:hover {
+    color: $ouiBreadCrumbHoverColor !important; // sass-lint:disable-line no-important
   }
 }
 
-.ouiBreadcrumbs:not(.ouiBreadcrumbs__inPopover) .ouiBreadcrumb--last {
+.ouiBreadcrumbs:not(.ouiBreadcrumbs__inPopover) .ouiBreadcrumbWrapper--last .ouiBreadcrumb {
   font-weight: $ouiFontWeightMedium;
 }
 
@@ -114,6 +111,16 @@
     margin-bottom: $ouiSizeXS; /* 1 */
     padding-left: $ouiSizeL - $ouiSizeXS + calc($ouiBreadcrumbSpacing / 2);
   }
+
+  // This targets the last breadcrumb wrapper and sets a different text color for the breadcrumb inside it.
+  &.ouiBreadcrumbWrapper--last .ouiBreadcrumb {
+    // TODO: remove important: https://github.com/opensearch-project/oui/issues/376
+    color: inherit !important; // sass-lint:disable-line no-important
+
+    &:hover {
+      color: inherit !important; // sass-lint:disable-line no-important
+    }
+  }
 }
 
 .ouiBreadcrumbWall {

diff --git a/src/components/breadcrumbs/breadcrumbs.tsx b/src/components/breadcrumbs/breadcrumbs.tsx
@@ -241,10 +241,8 @@ export const OuiBreadcrumbs: FunctionComponent<OuiBreadcrumbsProps> = ({
       'ouiBreadcrumb--truncate': truncate,
     });
 
-    let link;
-
-    if (!href && !onClick) {
-      link = (
+    const link =
+      !href && !onClick ? (
         <OuiInnerText>
           {(ref, innerText) => (
             <span
@@ -257,9 +255,7 @@ export const OuiBreadcrumbs: FunctionComponent<OuiBreadcrumbsProps> = ({
             </span>
           )}
         </OuiInnerText>
-      );
-    } else {
-      link = (
+      ) : (
         <OuiInnerText>
           {(ref, innerText) => (
             <OuiLink
@@ -275,19 +271,19 @@ export const OuiBreadcrumbs: FunctionComponent<OuiBreadcrumbsProps> = ({
           )}
         </OuiInnerText>
       );
-    }
-
-    let wrapper = <div className={breadcrumbWrapperClasses}>{link}</div>;
 
-    if (isFirstBreadcrumb) {
-      const breadcrumbWallClasses = classNames('ouiBreadcrumbWall', {
-        'ouiBreadcrumbWall--single': isLastBreadcrumb,
-      });
+    const breadcrumbWallClasses = classNames('ouiBreadcrumbWall', {
+      'ouiBreadcrumbWall--single': isFirstBreadcrumb && isLastBreadcrumb,
+    });
 
-      wrapper = <div className={breadcrumbWallClasses}>{wrapper}</div>;
-    }
+    const wrapper = <div className={breadcrumbWrapperClasses}>{link}</div>;
+    const wall = isFirstBreadcrumb ? (
+      <div className={breadcrumbWallClasses}>{wrapper}</div>
+    ) : (
+      wrapper
+    );
 
-    return <Fragment key={index}>{wrapper}</Fragment>;
+    return <Fragment key={index}>{wall}</Fragment>;
   });
 
   // Use the default object if they simply passed `true` for responsive
@@ -297,15 +293,16 @@ export const OuiBreadcrumbs: FunctionComponent<OuiBreadcrumbsProps> = ({
   // The max property collapses any breadcrumbs past the max quantity.
   // This is the same behavior we want for responsiveness.
   // So calculate the max value based on the combination of `max` and `responsive`
-  let calculatedMax: OuiBreadcrumbsProps['max'] = max;
-  // Set the calculated max to the number associated with the currentBreakpoint key if it exists
-  if (responsive && responsiveObject[currentBreakpoint as OuiBreakpointSize]) {
-    calculatedMax = responsiveObject[currentBreakpoint as OuiBreakpointSize];
-  }
-  // Final check is to make sure max is used over a larger breakpoint value
-  if (max && calculatedMax) {
-    calculatedMax = max < calculatedMax ? max : calculatedMax;
-  }
+
+  // First, calculate the responsive max value
+  const responsiveMax =
+    responsive && responsiveObject[currentBreakpoint as OuiBreakpointSize]
+      ? responsiveObject[currentBreakpoint as OuiBreakpointSize]
+      : null;
+
+  // Second, if both max and responsiveMax are set, use the smaller of the two. Otherwise, use the one that is set.
+  const calculatedMax: OuiBreadcrumbsProps['max'] =
+    max && responsiveMax ? Math.min(max, responsiveMax) : max || responsiveMax;
 
   const limitedBreadcrumbs = calculatedMax
     ? limitBreadcrumbs(breadcrumbElements, calculatedMax, breadcrumbs)