mia-platform / design-system Goto Github PK

The feature or bug you are proposing

Add a Collapse component.

The description of the bug or the rationale of your proposal

Add a component offering functionalities similar to Antd's Collapse: https://ant.design/components/collapse.
We need a Collapse component able to hide its children based on an activeKey, with an option for having no border and background color.

The feature or bug you are proposing

Move components prop definitions to separate files

The description of the bug or the rationale of your proposal

It would help with organization to have separate files for prop definitions, in order not to clutter component implementation files.

The feature or bug you are proposing

In order to keep the default pagination the table now requires to pass
pagination={...Table.pagination, ...customPartialPagination}

while it would be better to just pass
pagination={customPartialPagination}

The description of the bug or the rationale of your proposal

Easier developer experience in using the Table component

https://github.com/mia-platform/design-system/blob/71579f344b5036bbcd222d01ed5592f0379b7cd8/src/themes/schema/typography.ts#L31C70-L31C70

Current link is pointing to "font size" section;
correct link could be: https://developer.mozilla.org/en-US/docs/Web/CSS/font-style

The feature or bug you are proposing

Improve Table placeholder

The description of the bug or the rationale of your proposal

The table placeholder when loading does not fit the shape of the table and hides all parts of the table.
The ideal result would be a placeholder that performs optimistic updates on the fixed parts of the table (pagination, header...) and shows a loader only on the table content

Unit test (with jest and @testing-library/react) on components that renders the Table component might fail because of the following error:

    Hooks cannot be defined inside tests. Hook of type "afterAll" is nested within "...".

      1 | import {useMemo} from 'react'
    > 2 | import {Table} from '@mia-platform-internal/console-design-system-react'
        |                                                                         ^
      4 |

      at eventHandler (../../node_modules/react-scripts/node_modules/jest-circus/build/eventHandler.js:105:11)
      at Module.Object.<anonymous> (../../node_modules/@mia-platform-internal/console-design-system-react/dist/cjs/console-design-system-react.cjs:119:4403)
      at Object.<anonymous> (src/pages/Services/components/ServiceSidecars.jsx:3:73)
      at Object.<anonymous> (src/pages/Services/Detail.jsx:33:59)
      at Object.<anonymous> (src/containers/Services/ServiceDetail.jsx:49:60)
      at Object.<anonymous> (src/containers/Services/index.jsx:10:44)
      at src/containers/Design/Config/index.jsx:134:40

Weird enough, not all the tests returns this error but just one in a component that renders the Table as one of its children. It actually passes all the asserts, but returns this error nonetheless.

To solve this issue, I need to add the following import on the *.test.jsx file:

import { testUtils } from '@mia-platform-internal/console-design-system-react'

I assume because it imports the entire @testing-library/react. But I wish there was a way to avoid unnecessary imports.

Add the possibility to proxy className prop in each component

The feature or bug you are proposing

Pass className to a component

The description of the bug or the rationale of your proposal

For example, in the icon could be necessary to change the color depending on the hover state of the mouse. Despite it is possible with react, it is much more comfortable to address it using CSS

What do you think about?

The description of the bug or the rationale of your proposal

After upgrading to v0.1.16, some snapshots were updated due to random ids changing inside classnames

A snippet of code for replicating the issue or showing the proposal usage if applicable

Here is an example:

The expected result for your bug

Snapshots remain unchanged after upgrading to the newest DS version. Random ids inside class names might be a problem

Design system version: 0.1.16

The feature you are proposing

If the color prop is not set for an Icon, the default should be currentColor

The description rationale of your proposal

It might be helpful to have the currentColor as the default value so that icons inside buttons, menu items etc. automatically inherit the color based on the context in which they are placed.

design system version: 0.1.18

The feature or bug you are proposing

Build Storybook and run the command storybook-test whenever a merge request is submitted, ensuring components are built correctly and rendered as expected.

The description of the bug or the rationale of your proposal

Currently, components might be rendered incorrectly when updating dependencies.

The feature you are proposing

We can improve the spacing and overall balance of the feedback message: for example, we could increase the padding, the gap between the icon and the text, as well as the distance of the message from the top of the page.

A snippet of code for replicating the issue or showing the proposal usage if applicable

The expected result for your bug

Improved look

The feature or bug you are proposing

While using the Design System to migrate part of the UI in our application, we noticed that sometimes we use the data-testid attribute for testing purposes. However, the components in the DS does not expect this attribute, therefore we have to change our tests as well.

Do you think that the data-testid attribute will be included in the components?

A snippet of code for replicating the issue or showing the proposal usage if applicable

If I've a button component like the following:

<Button data-testid={'my-button-cancel'} onClick={onCancel} />

and a test (with Jest and React Testing Library) with this instruction:

userEvent.click(screen.getByTestId('my-button-cancel'))

The test will fail since the button won't have the data-testid atrtribute.

The expected result for your bug

For the Button component, as example:

export const Button = ({
  "data-testid": dataTestid,
  ...
}: ButtonProps): ReactElement  => {
 ...
return (
    <AntButton
      ...
      data-testid={dataTestid}
      ...
    />
  )
}

Your environment

node: v20.11.1

os: Ubuntu 22.04

The feature or bug you are proposing

Segmented control storybook seems to be broken in the github page

https://mia-platform.github.io/design-system/?path=/story/components-segmented-control--labeled-options

The expected result for your bug

Storybook to properly work

The feature or bug you are proposing

useFeedbackMessage supports afterClose

The description of the bug or the rationale of your proposal

In antd's message hook, it is possible to have an afterClose method, to be called after the message is closed, in a thenable interface, like this:

message[level](content, [duration]).then(afterClose)

This behaviour should be kept in the useFeedbackMessage hook as well.

A snippet of code for replicating the issue or showing the proposal usage if applicable

success({
      key: 'some key',
      message: 'some message',
      duration: 1
}).then(() =>{...})

The resulting error is: Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'then')

The expected result for your bug

afterClose can be called in thenable interface, e.g.
success(...).then(afterClose)

As a side note, in antd there is also an onClose parameter that can be passed to specify a function that will be called when the message is closed. Maybe that could be added as well

Design system version: 0.1.15

The feature or bug you are proposing

Currently, the repository includes Lodash specific packages, while it would be easier to use only Lodash bundled with es modules:
https://www.npmjs.com/package/lodash-es

https://github.com/mia-platform/design-system/blob/71579f344b5036bbcd222d01ed5592f0379b7cd8/src/themes/schema/typography.ts#L31C70-L31C70

Current link is pointing to "font size" section;
correct link could be: https://developer.mozilla.org/en-US/docs/Web/CSS/font-style

The feature or bug you are proposing

We should exclude from the bundle the following files:

• mocks
• stories
• tests

The description of the bug or the rationale of your proposal

Lowering the bundle size by excluding unnecessary files

The feature or bug you are proposing

Insertion of horizontal padding for cards in the loading state.

The description of the bug or the rationale of your proposal

When cards are in loading state, they have no horizontal padding between the skeleton and the edge of the card itself:

The expected result for your bug

The cards have, even in the loading state, a horizontal padding of 24px.

Your environment

node: v18.19.0

os: Ubuntu 22.04.3 LTS, 64-bit

The feature or bug you are proposing

Nested categories in the Menu component present an incorrect indentation

The description of the bug or the rationale of your proposal

Nested categories should be less indented than the items they contain (matching the style of items at the first nesting level).

The feature or bug you are proposing

Add the ability to customize colors for text, both in Card and Typography components as well as in external components using theme tokens.

The description of the bug or the rationale of your proposal

Fix Card component text colors. Allow new components to specify different colors for their embedded text.

The expected result for your bug

var(--palette-text-primary-main)

and

const {palette} = useTheme()
const color = palette?.text?.primary?.main

The feature or bug you are proposing

Creation of a Breadcrumb component to ease navigation.

The description of the bug or the rationale of your proposal

It would be nice to introduce an UI component to allow users to navigate between sections. This component should not be necessarily associated with the react-dom-router or represent a routing, but it could be also used to navigate inside a sub-section (e.g. inside a modal or a drawer).

The expected Breadcrumb component should have as input only an items property, which will contain a list of breadcrumb items to be displayed. The component should have the following props:

type BreadcrumbProps = {
    items: BreadcrumbItemType[]
}

Each specific item should be of type BreadcrumbItemType, structured this way

type BreadcrumbItemType = {
    icon?: ReactNode
    key?: string
    label?: ReactNode
    menu?: BreadcrumbItemMenu
    onClick?: (event: React.MouseEvent<Element, MouseEvent>) => void
}

If at least one of the icon, label or menu props are not undefined and contain a valid value, the component should display the associated value. If unspecified, the key property is set at the items index. If more than one item is present, the items should be displayed and divided by a fixed separator (>). As visible inside the type, each item contains an onClick prop to allow users to define the expected behavior to be executed when clicking.

If the item contains a valid menu value, the component should display an element containing a dropdown icon. If not specified otherwise, clicking this icon will open a dropdown, which will display the menu elements, if present, and a search bar, if specified.

The menu type should look like this:

type BreadcrumbItemMenu = {
    activeKey?: string
    items?: BreadcrumbItemMenuItem[]
    onDropdownVisibleChange?: (open: boolean) => void
    onSearch?: OnSearch
    open?: boolean
    searchAllowClear?: boolean
    searchPlaceholder?: string
    showSearch?: boolean
}

The showSearch, searchPlaceholder, searchAllowClear and onSearch props will give freedom to the user to specify if and how the search function should work. The open and onDropdownVisibleChange props allow to specify the dropdown behavior. If not specified, the internal state will be used to open and close the dropdown when clicking on the dropdown icon. The activeKey prop will be used to specify if one of the sub-items has already been selected. If so, its label will be displayed as the label of the parent BreadcrumbItem component, otherwise, the parent label will be displayed, if present. The items prop should contain a list of element with the following structure:

type BreadcrumbItemMenuItem = {
    icon?: ReactNode
    key?: string
    onClick?: (event: React.MouseEvent<Element, MouseEvent>) => void
    label: string
}

As for the previous type, if the key is unspecified, the items index will be used as key. Here as well the onClick property of each sub-item is left to be handled by a custom function.

Issue based on the comment below

          I'd map this prop to the isClosable flag.

Originally posted by @fredmaggiowski in #334 (comment)

Closing modals by clicking outside of it is a basic feature that provides ease-of-use for most of the modals we use. I understand some modals shouldn't be closable that easy but I'd not limit UX to have to click the X in the header (or the close button).

I'd consider rethinking this design choice.

The feature or bug you are proposing

The modal component have three different sizes: small, large and fullscreen. Since it doesn't use dynamic height, we might run in cases where we would need an intermediate size to work with to have enough space to include the content properly.

The description of the bug or the rationale of your proposal

The feature would include a size medium, where the width would be the 50-55% of the viewport and height would be reasonably the 60-65% of the viewport.

The feature you are proposing

Improvement of alignment inside table columns

The description of the bug or the rationale of your proposal

When using the align property on table columns, it sets the text-align CSS property value, which sets the horizontal alignment of the inline-level content inside a block element or table-cell box. This means that non-inline elements (e.g. divs) do not get aligned correctly.

The expected result for your bug

The table cell could support a wrapper or something similar that makes its content "inline-block"

Design system version: 0.1.13

The description of the bug or the rationale of your proposal

Table actions should be 24x24 Buttons, containing 16x16 Icons. Right now, actions are 24x24 icons.

The feature or bug you are proposing

If I need to use this button as submit button of a form, it's needed to set the html type attribute to submit and form attribute to the id of the form.

So, it would be something like:

<Button htmlType="submit" htmlForm="my-form-id">{"something"}</Button>

The feature or bug you are proposing

Add a Getting Started page, that can be read from Storybook and from the repository to understand the basic concepts behind:

• ThemeProvider and themes configuration
• Test utils and configuration

The description of the bug or the rationale of your proposal

Allow new users to understand how to properly use Design System components

The feature or bug you are proposing

Add a Divider component.

The description of the bug or the rationale of your proposal

Add a component offering functionalities similar to Antd's Divider: https://ant.design/components/divider.
For now, we need a basic divider, but most of the Antd interface could be implemented:

{
  className: string
  dashed: boolean
  orientation: 'left' | 'right' | 'center'
  orientationMargin: string | number
  plain: boolean
  type: 'horizontal' | 'vertical'
}

The feature or bug you are proposing

It is currently impossible to use the table with data fetched asynchronously and pagination without explicitly set pagination.current and keep track of it in the component using it.

The description of the bug or the rationale of your proposal

With such configuration, when I change page I correctly get the page change callback, I properly download data and provide it back to the table which re-renders but sets pagination based to the defaultCurrent (so 1) which is meant to be read only on mount, the final user of this design system is not remounting the Table, therefore this should be addressed in some way.

A snippet of code for replicating the issue or showing the proposal usage if applicable

const MyComponent = ({...}) => {
  ...

  const onChangePage = useCallback((page) => requestGet(`?page=${page}&per_page=${DEFAULT_PAGE_SIZE}`), [requestGet])

  const paginationProps = useMemo(() => ({
    ...Table.pagination,
    defaultCurrent: 1, <-- when loading changes the table is remounted from scratch and this overwrites the current page
    defaultPageSize: DEFAULT_PAGE_SIZE,
    onChange: onChangePage,
    total: totalItems,
    showSizeChanger: false
  }), [onChangePage, totalItems])

  return (
    requestError ?
      errorFeedback :
        <Table
          columns={columns}
          data={historySnapshots}
          isLoading={isLoading}
          pagination={paginationProps}
          rowKey='date'
          scroll={scroll}
          size='small'
        />
  )
}

The expected result for your bug

Since the Table knows the current page (it is provided by the pagination.onChange callback) it should keep track of it between loading changes in order to provide a coherent UX to the final user

Your environment

node: 18
os: macos

The package.json file declares a exposed index.d.ts file in dist folder that instead does not exist.

Can you add the prop for table rowClassName from antd that let add classnames on row, like adding a different background in base of state