Feature/table

adr/001-component-library.md

@@ -0,0 +1,65 @@
+# Introduce component library
+
+- Status: Accepted
+- Deciders: Team
+- Date: 17.10.2024
+
+## Result
+
+A1: Component library is introduced, firstly as just a folder in our current setup, adding yarn workspaces or similar requires more research and testing
+
+## Problem context
+
+Today our UI components are tightly coupled to the app in which they are rendered.
+
+This leads to several issues:
+
+- Makes it hard to do component testing outside a fully rendered app.
+  - See tests in `test/e2e/integration` for examples of how we have most of our tests in integration tests because components need to be running inside an app to work.
+  - See `src/test/renderWithProviders.tsx` for examples of how much scaffolding we need to do to run component tests currently. This also leads to very slow component tests.
+- Makes refactoring the app framework a lot harder.
+  - If we search for `useNodeItem` in src/layout we get over 100 hits. If we make changes or remove hooks like this, every single UI component must be updated.
+- Leads to unclear interfaces between UI components and the app framework.
+  - See: `src/layout/Button/ButtonComponent.tsx`. This component references `node.parent` which confuses the role and responsibility of the button component.
+- Makes developing UI components complex without deep understanding of the application.
+- Enables sharing of pure components (docs, Studio, storybook)
+
+## Decision drivers
+
+A list of decision drivers. These are points which can differ in importance. If a point is "nice to have" rather than
+"need to have", then prefix the description.
+
+- B1: UI components should only receive data to display, and notify the app when data is changed.
+- B2: UI components should live in a separate folder from the app itself, and have no dependencies to the app.
+- B3: UI components should live in a lib separately to the src folder to have stricter control of dependencies.
+
+## Alternatives considered
+
+List the alternatives that were considered as a solution to the problem context.
+
+- A1: Simply put a new folder inside the src folder.
+- A2: Use yarn workspaces to manage the library separately from the src folder.
+- A3: Set up NX.js to manage our app and libraries.
+
+## Pros and cons
+
+List the pros and cons with the alternatives. This should be in regards to the decision drivers.
+
+### A1
+
+- Good because B1 and B2 is covered
+- Allows us to really quickly get started with a component library
+- Bad, because it does not fulfill B3. If we simply use a new folder, it will be up to developers to enforce the rules of the UI components, like the avoiding dependencies to the app.
+
+### A2
+
+- Good, because this alternative adheres to B1, B2 and B3.
+- This way our libs would live separately to the app, and it would be obvious that it is a lib.
+- The con is that it takes more setup.
+
+### A3
+
+- Good, because this alternative adheres to B1, B2 and B3.
+- This way our libs would live separately to the app, and it would be obvious that it is a lib.
+- Also gives us powerful monorepo tooling.
+- Bad because it takes a lot more time to set up, and might be overkill before we have decided to integrate frontend and backend into monorepo.

src/app-components/readme.md

@@ -0,0 +1,11 @@
+## IMPORTANT
+
+Components in this folder should be 'dumb', meaning the should have no knowledge about the containing app.
+
+They should implement three things:
+
+1. Receive and display data in the form of props.
+2. Report data changes in a callback.
+3. Receive and display the validity of the data.
+
+These components will form the basis of our future component library, and will enable refactoring of the frontend app.

src/app-components/table/Table.module.css

@@ -0,0 +1,88 @@
+.table {
+  width: 100%;
+}
+
+.buttonContainer {
+  display: flex;
+  justify-content: end;
+  gap: var(--fds-spacing-2);
+}
+
+.mobileTable {
+  display: block;
+}
+
+.mobileTable caption {
+  display: block;
+}
+
+.mobileTable thead {
+  display: none;
+}
+
+.mobileTable th {
+  display: block;
+  border: none;
+}
+
+.mobileTable tbody,
+.mobileTable tr {
+  display: block;
+}
+
+.mobileTable td {
+  display: flex;
+  flex-direction: column;
+  border: none;
+  padding: var(--fds-spacing-2) 0;
+}
+
+.mobileTable td .contentWrapper {
+  display: flex;
+  flex-direction: row;
+  align-items: center;
+  width: 100%;
+}
+
+.mobileTable tbody tr:last-child {
+  border-bottom: 2px solid var(--fds-semantic-border-neutral-default);
+}
+
+.mobileTable tbody tr:first-child {
+  border-top: 2px solid var(--fds-semantic-border-neutral-default);
+}
+
+.mobileTable tr {
+  border-bottom: 1px solid var(--fds-semantic-border-neutral-default);
+  padding-top: var(--fds-spacing-3);
+  padding-bottom: var(--fds-spacing-3);
+}
+
+.mobileTable tr:hover td {
+  background-color: unset;
+}
+
+.mobileTable td[data-header-title]:not([data-header-title=''])::before,
+.mobileTable th[data-header-title]:not([data-header-title=''])::before {
+  content: attr(data-header-title);
+  display: block;
+  text-align: left;
+  font-weight: 500;
+}
+
+.mobileTable td .buttonContainer {
+  justify-content: start;
+}
+
+.visuallyHidden {
+  border: none;
+  padding: 0;
+  margin: 0;
+  position: absolute;
+  height: 1px;
+  width: 1px;
+  overflow: hidden;
+  clip: rect(1px 1px 1px 1px);
+  clip-path: inset(50%);
+  white-space: nowrap;
+}

src/app-components/table/Table.test.tsx

@@ -0,0 +1,180 @@
+// AppTable.test.tsx
+import React from 'react';
+
+import { fireEvent, render, screen } from '@testing-library/react';
+
+import { AppTable } from 'src/app-components/table/Table';
+
+// Sample data
+const data = [
+  { id: 1, name: 'Alice', date: '2023-10-05', amount: 100 },
+  { id: 2, name: 'Bob', date: '2023-10-06', amount: 200 },
+];
+
+// Columns configuration
+const columns = [
+  { header: 'Name', accessors: ['name'] },
+  { header: 'Date', accessors: ['date'] },
+  { header: 'Amount', accessors: ['amount'] },
+];
+
+// Action buttons configuration
+const actionButtons = [
+  {
+    onClick: jest.fn(),
+    buttonText: 'Edit',
+    icon: null,
+  },
+  {
+    onClick: jest.fn(),
+    buttonText: 'Delete',
+    icon: null,
+  },
+];
+
+describe('AppTable Component', () => {
+  test('renders table with correct headers', () => {
+    render(
+      <AppTable
+        data={data}
+        columns={columns}
+      />,
+    );
+    expect(screen.getByText('Name')).toBeInTheDocument();
+    expect(screen.getByText('Date')).toBeInTheDocument();
+    expect(screen.getByText('Amount')).toBeInTheDocument();
+  });
+
+  test('renders correct number of rows', () => {
+    render(
+      <AppTable
+        data={data}
+        columns={columns}
+      />,
+    );
+    const rows = screen.getAllByRole('row');
+    expect(rows.length).toBe(data.length + 1); // +1 for header row
+  });
+
+  test('renders action buttons when provided', () => {
+    render(
+      <AppTable
+        data={data}
+        columns={columns}
+        actionButtons={actionButtons}
+      />,
+    );
+    expect(screen.getAllByText('Edit').length).toBe(data.length);
+    expect(screen.getAllByText('Delete').length).toBe(data.length);
+  });
+
+  test('correctly formats dates in cells', () => {
+    render(
+      <AppTable
+        data={data}
+        columns={columns}
+      />,
+    );
+    expect(screen.getByText('05.10.2023')).toBeInTheDocument();
+    expect(screen.getByText('06.10.2023')).toBeInTheDocument();
+  });
+
+  test('uses renderCell function when provided', () => {
+    const columnsWithRenderCell = [
+      ...columns,
+      {
+        header: 'Custom',
+        accessors: ['name', 'amount'],
+        renderCell: (values) => `Name: ${values[0]}, Amount: ${values[1]}`,
+      },
+    ];
+    render(
+      <AppTable
+        data={data}
+        columns={columnsWithRenderCell}
+      />,
+    );
+    expect(screen.getByText('Name: Alice, Amount: 100')).toBeInTheDocument();
+    expect(screen.getByText('Name: Bob, Amount: 200')).toBeInTheDocument();
+  });
+
+  test('calls action button onClick when clicked', () => {
+    const onClickMock = jest.fn();
+    const actionButtonsMock = [
+      {
+        onClick: onClickMock,
+        buttonText: 'Edit',
+        icon: null,
+      },
+    ];
+
+    render(
+      <AppTable
+        data={data}
+        columns={columns}
+        actionButtons={actionButtonsMock}
+      />,
+    );
+
+    const editButtons = screen.getAllByText('Edit');
+    fireEvent.click(editButtons[0]);
+    expect(onClickMock).toHaveBeenCalledWith(0, data[0]);
+
+    fireEvent.click(editButtons[1]);
+    expect(onClickMock).toHaveBeenCalledWith(1, data[1]);
+
+    expect(onClickMock).toHaveBeenCalledTimes(2);
+  });
+
+  test('renders "-" when cell values are null or undefined', () => {
+    const dataWithNull = [
+      { id: 1, name: 'Alice', date: null, amount: 100 },
+      { id: 2, name: 'Bob', date: '2023-10-06', amount: 200 },
+    ];
+    render(
+      <AppTable
+        data={dataWithNull}
+        columns={columns}
+      />,
+    );
+    expect(screen.getAllByText('-').length).toBe(1);
+  });
+
+  test('does not render action buttons column when actionButtons is not provided', () => {
+    render(
+      <AppTable
+        data={data}
+        columns={columns}
+      />,
+    );
+    const headerCells = screen.getAllByRole('columnheader');
+    expect(headerCells.length).toBe(columns.length);
+  });
+
+  test('renders extra header cell when actionButtons are provided', () => {
+    render(
+      <AppTable
+        data={data}
+        columns={columns}
+        actionButtons={actionButtons}
+      />,
+    );
+    const headerCells = screen.getAllByRole('columnheader');
+    expect(headerCells.length).toBe(columns.length + 1);
+  });
+
+  test('non-date values are not changed by formatIfDate', () => {
+    const dataWithNonDate = [
+      { id: 1, name: 'Alice', date: 'Not a date', amount: 100 },
+      { id: 2, name: 'Bob', date: 'Also not a date', amount: 200 },
+    ];
+    render(
+      <AppTable
+        data={dataWithNonDate}
+        columns={columns}
+      />,
+    );
+    expect(screen.getByText('Not a date')).toBeInTheDocument();
+    expect(screen.getByText('Also not a date')).toBeInTheDocument();
+  });
+});