fix: review 2

sbb-design-systems · Aug 23, 2024 · d8c3794 · d8c3794
1 parent d75569d
commit d8c3794
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 44 deletions.
diff --git a/src/elements/paginator/paginator.scss b/src/elements/paginator/paginator.scss
@@ -48,13 +48,12 @@
 }
 
 .sbb-paginator__pages {
+  @include sbb.list-reset;
+
   display: flex;
   align-items: center;
   justify-content: center;
   gap: var(--sbb-spacing-fixed-1x);
-  list-style: none;
-  margin: 0;
-  padding: 0;
   user-select: none;
   -webkit-tap-highlight-color: transparent;
 }

diff --git a/src/elements/paginator/paginator.ts b/src/elements/paginator/paginator.ts
@@ -70,7 +70,7 @@ export class SbbPaginatorElement extends SbbNegativeMixin(LitElement) {
   @property({ attribute: 'page-index', type: Number })
   public set pageIndex(value: number) {
     const previousPageIndex = this._pageIndex;
-    this._pageIndex = this._calculatePageIndex(value);
+    this._pageIndex = this._validatePageIndex(value);
     if (previousPageIndex !== this._pageIndex) {
       this._pageChanged.emit({ previousPageIndex, currentPageIndex: value });
     }
@@ -136,8 +136,8 @@ export class SbbPaginatorElement extends SbbNegativeMixin(LitElement) {
   }
 
   /** Evaluate `pageIndex` by excluding edge cases. */
-  private _calculatePageIndex(pageIndex: number): number {
-    if (isNaN(pageIndex) || pageIndex < 0 || pageIndex > this._numberOfPages()) {
+  private _validatePageIndex(pageIndex: number): number {
+    if (isNaN(pageIndex) || pageIndex < 0 || pageIndex > this._numberOfPages) {
       return 0;
     }
     return pageIndex;
@@ -147,7 +147,7 @@ export class SbbPaginatorElement extends SbbNegativeMixin(LitElement) {
    * Calculates the current number of pages based on the `length` and the `pageSize`;
    * value must be rounded up (e.g. `length = 21` and `pageSize = 10` means 3 pages).
    */
-  private _numberOfPages(): number {
+  private get _numberOfPages(): number {
     if (!this.pageSize) {
       return 0;
     }
@@ -161,7 +161,7 @@ export class SbbPaginatorElement extends SbbNegativeMixin(LitElement) {
    *  - if there are more than `MAX_PAGE_NUMBERS_DISPLAYED` other pages, ellipsis button must be used.
    */
   private _getVisiblePagesIndex(): (number | null)[] {
-    const totalPages: number = this._numberOfPages();
+    const totalPages: number = this._numberOfPages;
     const currentPageIndex: number = this.pageIndex;
 
     if (totalPages <= MAX_PAGE_NUMBERS_DISPLAYED + 2) {
@@ -250,7 +250,7 @@ export class SbbPaginatorElement extends SbbNegativeMixin(LitElement) {
             id="sbb-paginator-next-page"
             aria-label=${i18nNextPage[this._language.current]}
             icon-name="chevron-small-right-small"
-            ?disabled=${this.pageIndex === this._numberOfPages() - 1}
+            ?disabled=${this.pageIndex === this._numberOfPages - 1}
             @click=${() => this._changePage(this.pageIndex + 1)}
           ></sbb-mini-button>
         </sbb-mini-button-group>

diff --git a/src/elements/paginator/paginator.visual.spec.ts b/src/elements/paginator/paginator.visual.spec.ts
@@ -11,16 +11,17 @@ import {
 import './paginator.js';
 
 describe('sbb-paginator', () => {
-  describeViewports({ viewports: ['small', 'medium'] }, () => {
+  describeViewports({ viewports: ['zero', 'medium'] }, () => {
     for (const negative of [false, true]) {
       const wrapperStyle = {
+        minHeight: '300px',
         backgroundColor: negative ? 'var(--sbb-color-black)' : undefined,
         focusOutlineDark: negative,
       };
 
       for (const state of [visualDiffDefault, visualDiffFocus]) {
         it(
-          `${state.name} negative=${negative}`,
+          `negative=${negative} state=${state.name}`,
           state.with(async (setup) => {
             await setup.withFixture(
               html` <sbb-paginator
@@ -43,7 +44,7 @@ describe('sbb-paginator', () => {
       for (const state of [visualDiffActive, visualDiffHover]) {
         for (const selected of [false, true]) {
           it(
-            `${state.name} negative=${negative} selected=${selected}`,
+            `negative=${negative} state=${state.name} selected=${selected}`,
             state.with(async (setup) => {
               await setup.withFixture(
                 html` <sbb-paginator
@@ -68,7 +69,7 @@ describe('sbb-paginator', () => {
 
       for (const pageIndex of [0, 2, 5, 7, 9]) {
         it(
-          `pageIndex=${pageIndex} negative=${negative}`,
+          `negative=${negative} pageIndex=${pageIndex}`,
           visualDiffDefault.with(async (setup) => {
             await setup.withFixture(
               html` <sbb-paginator
@@ -82,36 +83,24 @@ describe('sbb-paginator', () => {
         );
       }
 
-      it(
-        `pageSizeOptions negative=${negative}`,
-        visualDiffDefault.with(async (setup) => {
-          const pageSizeOptions = [10, 20, 50];
-          await setup.withFixture(
-            html` <sbb-paginator
-              length="50"
-              page-size="4"
-              .pageSizeOptions="${pageSizeOptions}"
-              ?negative=${negative || nothing}
-            ></sbb-paginator>`,
-            wrapperStyle,
-          );
-        }),
-      );
-
-      it(
-        `size=s negative=${negative}`,
-        visualDiffDefault.with(async (setup) => {
-          await setup.withFixture(
-            html` <sbb-paginator
-              length="50"
-              page-size="4"
-              size="s"
-              ?negative=${negative || nothing}
-            ></sbb-paginator>`,
-            wrapperStyle,
-          );
-        }),
-      );
+      for (const size of ['s', 'm']) {
+        it(
+          `negative=${negative} size=${size} pageSizeOptions`,
+          visualDiffDefault.with(async (setup) => {
+            const pageSizeOptions = [10, 20, 50];
+            await setup.withFixture(
+              html` <sbb-paginator
+                length="50"
+                page-size="4"
+                size=${size}
+                .pageSizeOptions="${pageSizeOptions}"
+                ?negative=${negative || nothing}
+              ></sbb-paginator>`,
+              wrapperStyle,
+            );
+          }),
+        );
+      }
     }
   });
 });
diff --git a/...sual-regression-app/src/components/test-case/test-title-chip-list/test-title-chip-list.ts b/...sual-regression-app/src/components/test-case/test-title-chip-list/test-title-chip-list.ts
@@ -9,7 +9,7 @@ import style from './test-title-chip-list.scss?lit&inline';
  * - simple => "key=value" patterns
  * - complex => "key=( key=value-... )
  */
-const paramsRegex = /(?<complex>[a-zA-Z]*=\(.*\))|(?<simple>[a-zA-Z]+=[a-zA-Z]*)/gm;
+const paramsRegex = /(?<complex>[a-zA-Z]*=\(.*\))|(?<simple>[a-zA-Z]+=[a-zA-Z0-9]*)/gm;
 
 type DescribeEachItem = {
   key: string;