Merge branch 'main' into LOOM-1368

Skyscanner · Apr 22, 2024 · 9c65bbd · 9c65bbd
2 parents 6fdcca2 + adc0f04
commit 9c65bbd
Show file tree

Hide file tree

Showing 7 changed files with 8 additions and 62 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -135,7 +135,10 @@
           {
             "propName": "className",
             "message":
-              "Prop {{prop}} is forbidden. See https://github.com/Skyscanner/eslint-plugin-rules?tab=readme-ov-file#forbid-component-props"
+              "Prop {{prop}} is forbidden. See https://github.com/Skyscanner/eslint-plugin-rules?tab=readme-ov-file#forbid-component-props",
+            "allowedFor": [
+              "InnerContainer" // allowed for a prop that is passed down in the BpkMobileScrollContainer to a generated element
+            ]
           }
         ]
       }

diff --git a/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer-test.js b/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer-test.js
@@ -120,7 +120,7 @@ describe('BpkMobileScrollContainer', () => {
         expect(computeScrollIndicatorClassName(null)).toBeNull();
       });
 
-      it('should return left className and custom leadingIndicatorClassName if scrolling left is possible', () => {
+      it('should return custom leadingIndicatorClassName if scrolling left is possible', () => {
         const scrollerEl = makeMockScroller(10, 200, 200);
 
         expect(
@@ -130,12 +130,11 @@ describe('BpkMobileScrollContainer', () => {
             'custom-trailing-class-name',
           ),
         ).toEqual([
-          'bpk-mobile-scroll-container--left-indicator',
           'custom-leading-class-name',
         ]);
       });
 
-      it('should return right className and custom trailingIndicatorClassName if scrolling right is possible', () => {
+      it('should return custom trailingIndicatorClassName if scrolling right is possible', () => {
         const scrollerEl = makeMockScroller(0, 250, 200);
 
         expect(
@@ -145,12 +144,11 @@ describe('BpkMobileScrollContainer', () => {
             'custom-trailing-class-name',
           ),
         ).toEqual([
-          'bpk-mobile-scroll-container--right-indicator',
           'custom-trailing-class-name',
         ]);
       });
 
-      it('should return right and left className plus custom leadingIndicatorClassName and custom trailingIndicatorClassName if scrolling both right and left is possible', () => {
+      it('should return custom leadingIndicatorClassName and custom trailingIndicatorClassName if scrolling both right and left is possible', () => {
         const scrollerEl = makeMockScroller(10, 250, 200);
 
         const classNames = computeScrollIndicatorClassName(
@@ -159,12 +157,6 @@ describe('BpkMobileScrollContainer', () => {
           'custom-trailing-class-name',
         );
 
-        expect(classNames).toContain(
-          'bpk-mobile-scroll-container--left-indicator',
-        );
-        expect(classNames).toContain(
-          'bpk-mobile-scroll-container--right-indicator',
-        );
         expect(classNames).toContain('custom-leading-class-name');
         expect(classNames).toContain('custom-trailing-class-name');
       });

diff --git a/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer.js b/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer.js
@@ -58,19 +58,7 @@ const computeScrollIndicatorClassName = (
   const scrollValue = rtl ? -Math.floor(scrollLeft) : Math.ceil(scrollLeft);
   const showLeadingIndicator = scrollValue > 0;
   const showTrailingIndicator = scrollValue < scrollWidth - offsetWidth;
-  const showLeftIndicator = rtl ? showTrailingIndicator : showLeadingIndicator;
-  const showRightIndicator = rtl ? showLeadingIndicator : showTrailingIndicator;
 
-  if (showLeftIndicator) {
-    classNames.push(
-      getClassName('bpk-mobile-scroll-container--left-indicator'),
-    );
-  }
-  if (showRightIndicator) {
-    classNames.push(
-      getClassName('bpk-mobile-scroll-container--right-indicator'),
-    );
-  }
   if (showLeadingIndicator && leadingIndicatorClassName) {
     classNames.push(leadingIndicatorClassName);
   }
@@ -231,8 +219,6 @@ class BpkMobileScrollContainer extends Component<Props, State> {
             ref={(el) => {
               this.innerEl = el;
             }}
-            // TODO: className to be removed
-            // eslint-disable-next-line @skyscanner/rules/forbid-component-props
             className={getClassName('bpk-mobile-scroll-container__inner')}
           >
             {children}

diff --git a/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer.module.scss b/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer.module.scss
@@ -23,18 +23,6 @@
   position: relative;
   overflow: hidden;
 
-  &--left-indicator {
-    @include scroll-indicators.bpk-scroll-indicator-left(
-      tokens.$bpk-color-white
-    );
-  }
-
-  &--right-indicator {
-    @include scroll-indicators.bpk-scroll-indicator-right(
-      tokens.$bpk-color-white
-    );
-  }
-
   &__scroller {
     white-space: nowrap;
     overflow-x: auto;

diff --git a/packages/bpk-component-rtl-toggle/src/BpkRtlToggle-test.tsx b/packages/bpk-component-rtl-toggle/src/BpkRtlToggle-test.tsx
@@ -25,9 +25,4 @@ describe('BpkRtlToggle', () => {
     const { asFragment } = render(<BpkRtlToggle />);
     expect(asFragment()).toMatchSnapshot();
   });
-
-  it('should render correctly with the className prop', () => {
-    const { asFragment } = render(<BpkRtlToggle className="foo" />);
-    expect(asFragment()).toMatchSnapshot();
-  });
 });
diff --git a/packages/bpk-component-rtl-toggle/src/BpkRtlToggle.tsx b/packages/bpk-component-rtl-toggle/src/BpkRtlToggle.tsx
@@ -41,9 +41,7 @@ type State = {
   direction: string;
 };
 
-type Props = {
-  className?: string | null;
-};
+type Props = {};
 
 type InteractionEvents = KeyboardEvent | MouseEvent | TouchEvent;
 
@@ -82,16 +80,12 @@ class BpkRtlToggle extends Component<Props, State> {
   };
 
   render() {
-    const { className } = this.props;
     const onOrOff = this.state.direction === DIRECTIONS.RTL ? 'off' : 'on';
 
     return (
       <BpkButtonLink
         title="Keyboard Shortcut: ctrl + cmd + r"
         onClick={this.toggleRtl}
-      // TODO: className to be removed
-      // eslint-disable-next-line @skyscanner/rules/forbid-component-props
-        className={className}
       >
         RTL {onOrOff}
       </BpkButtonLink>

diff --git a/packages/bpk-component-rtl-toggle/src/__snapshots__/BpkRtlToggle-test.tsx.snap b/packages/bpk-component-rtl-toggle/src/__snapshots__/BpkRtlToggle-test.tsx.snap
@@ -11,15 +11,3 @@ exports[`BpkRtlToggle should render correctly 1`] = `
   </button>
 </DocumentFragment>
 `;
-
-exports[`BpkRtlToggle should render correctly with the className prop 1`] = `
-<DocumentFragment>
-  <button
-    class="bpk-link foo"
-    title="Keyboard Shortcut: ctrl + cmd + r"
-    type="button"
-  >
-    RTL on
-  </button>
-</DocumentFragment>
-`;