gravity-ui / rfc

Gravity RFC is a process for proposing and implementing changes in our ecosystem
MIT License
4 stars 0 forks source link

Opening new tabs using the mouse wheel/cmd + clicking on a row in the Table component #18

Open Radomir-Drukh opened 2 months ago

Radomir-Drukh commented 2 months ago

Objective

Currently, Table does not have the ability to emulate the behavior of ordinary links when clicking on a row - namely, the ability to open a new tab without focusing on it. Modern browsers do not allow you to process the click event via onRowClick or onRowMouseDown using JS and do the necessary action, and the Table component does not provide other convenient options to achieve this. Despite this, going through the list of entities and opening several tabs at once to look at them later in turn is a fairly common scenario.

Solution Proposal

Add a util function or HOC, which will allow you to "wrap" an entire row into the desired link, while allowing you to interact with interactive elements in cells.

Proposed implementation

LinkTableUtils.tsx ```typescript import {Link, TableColumnConfig, TableProps} from '@gravity-ui/uikit'; import {cn} from 'src/helpers'; import _get from 'lodash/get'; import _has from 'lodash/has'; import cx from 'classnames'; import {ReactNode} from 'react'; import './LinkTableUtils.scss'; const b = cn('link-table-utils'); export type TableColumnConfigWithRowLink = Omit, 'meta'> & { meta: Omit['meta']>, 'isInteractive'> & { isInteractive: boolean; }; }; export const defaultLinkRowClassName = b('row'); export const interactiveElementClassName = b('interactive-element'); export const getDefaultLinkRowDescriptor = () => ({classNames: [defaultLinkRowClassName]}); export const prepareGetRowDescriptor = ( originalGetRowDescriptor: NonNullable['getRowDescriptor']>, ) => { const newGetRowDescriptor: TableProps['getRowDescriptor'] = (...args) => { const descriptor = originalGetRowDescriptor(...args); return { ...descriptor, classNames: [cx(descriptor?.classNames, defaultLinkRowClassName)], }; }; return newGetRowDescriptor; }; export const prepareLinkColumns = ( columns: TableColumnConfigWithRowLink[], getLinkProps: (entity: T, index: number) => {href?: string; target?: string}, ): TableColumnConfig[] => { const preparedColumns: TableColumnConfig[] = []; columns.forEach((column) => { const meta = column.meta; if (meta.isInteractive) { const newClassName = cx(column.className, b('interactive-cell')); preparedColumns.push({ ...column, className: newClassName, }); } else { preparedColumns.push(column); } }); preparedColumns.push({ id: '_row-link-internal-column', name: '', className: b('link-internal-column'), template: (entity, index) => { const linkProps = getLinkProps(entity, index); if (linkProps.href) { return ( ); } return null; }, }); return preparedColumns; }; ```
LinkTableUtils.scss ```scss .link-table-utils { $block: &; &__row { // new context, transform instead of position relative because of safari transform: translateZ(0); &:hover { background-color: var(--g-color-base-simple-hover-solid); } } &__main-link-cell { vertical-align: top; &::before { position: absolute; inset: 0; content: ''; } } &__interactive-element { z-index: 1; position: relative; } &__interactive-cell { >* { @extend #{$block}__interactive-element; } } &__link-internal-column { overflow: hidden; width: 0 !important; max-width: 0 !important; padding: 0 !important; border-width: 0 !important; font-size: 0 !important; } } ```
Basic usage ```typescript // Add to TableColumnConfig meta: { isInteractive: false, }, // Change props columns={prepareLinkColumns(tableColumns, getLinkProps)} getRowDescriptor={getDefaultLinkRowDescriptor} ```

Definition of done

Utilities or HOC are added to gravity uikit.

SeqviriouM commented 2 months ago

@beliarh @kseniya57 what do you think?

SeqviriouM commented 2 months ago

@Radomir-Drukh do you mean the component https://github.com/gravity-ui/table?

Radomir-Drukh commented 2 months ago
Radomir-Drukh commented 2 months ago

@SeqviriouM but if this implementation can be useful for https://github.com/gravity-ui/table as well, I don't mind adding it to both places

Radomir-Drukh commented 2 months ago

Made a quick HOC variant

LinkTableUtils.tsx ```typescript import { getComponentName, Link, TableColumnConfig, TableDataItem, TableProps, } from '@gravity-ui/uikit'; import React from 'react'; import {cn} from 'src/helpers'; import _get from 'lodash/get'; import _has from 'lodash/has'; import cx from 'classnames'; import './LinkTableUtils.scss'; const b = cn('link-table-utils'); export type TableColumnConfigWithRowLink = Omit, 'meta'> & { meta: Omit['meta']>, 'isInteractive'> & { isInteractive: boolean; }; }; export const defaultLinkRowClassName = b('row'); export const interactiveElementClassName = b('interactive-element'); export const getDefaultLinkRowDescriptor = () => ({classNames: [defaultLinkRowClassName]}); export const prepareGetRowDescriptor = ( originalGetRowDescriptor: TableProps['getRowDescriptor'], ) => { if (!originalGetRowDescriptor) { return getDefaultLinkRowDescriptor; } const newGetRowDescriptor: TableProps['getRowDescriptor'] = (...args) => { const descriptor = originalGetRowDescriptor(...args); return { ...descriptor, classNames: [cx(descriptor?.classNames, defaultLinkRowClassName)], }; }; return newGetRowDescriptor; }; export const prepareLinkColumns = ( columns: TableColumnConfigWithRowLink[], getLinkProps: (entity: T, index: number) => {href?: string; target?: string}, ): TableColumnConfig[] => { const preparedColumns: TableColumnConfig[] = []; columns.forEach((column) => { const meta = column.meta; if (meta.isInteractive) { const newClassName = cx(column.className, b('interactive-cell')); preparedColumns.push({ ...column, className: newClassName, }); } else { preparedColumns.push(column); } }); preparedColumns.push({ id: '_row-link-internal-column', name: '', className: b('link-internal-column'), template: (entity, index) => { const linkProps = getLinkProps(entity, index); if (linkProps.href) { return ( ); } return null; }, }); return preparedColumns; }; export function withTableRowLink( TableComponent: React.ComponentType & E>, getLinkProps: (entity: I, index: number) => {href?: string; target?: string}, ): React.ComponentType< Omit, 'columns'> & {columns: TableColumnConfigWithRowLink[]} & E > { const componentName = getComponentName(TableComponent); const displayName = `withTableRowLink(${componentName})`; return class extends React.Component< Omit, 'columns'> & {columns: TableColumnConfigWithRowLink[]} & E > { static displayName = displayName; render() { const {columns, getRowDescriptor, ...restTableProps} = this.props; return ( , 'columns'> & E)} columns={prepareLinkColumns(columns, getLinkProps)} getRowDescriptor={prepareGetRowDescriptor(getRowDescriptor)} /> ); } }; } ```
SeqviriouM commented 2 months ago

@Raubzeug @amje @korvin89 what do you think about proposal?

amje commented 2 months ago

@Radomir-Drukh After releasing @gravity-ui/table we are planning to remove all logic from our Table and keep only "view" components in uikit. So adding new functionality to the Table I see irrational if we delete it afterwards

Radomir-Drukh commented 2 months ago

@amje

A couple questions:

1) Do you have described functionality already added to @gravity-ui/table? If not, this proposal can be used to improve @gravity-ui/table

2) As far as I know there are a lot of instances of using Table from uikit (500+ only in Arcadia). Do you expect all teams to migrate to @gravity-ui/table just to add convenient links behaviour?

Radomir-Drukh commented 2 months ago

@amje @Raubzeug @amje @korvin89 @SeqviriouM

Raubzeug commented 2 months ago

@Radomir-Drukh I agree with @amje. All new features should be implemented to @gravity-ui/table.

@beliarh what do you think about this proposal? It seems this can be easily done with getRowModel.

beliarh commented 2 months ago

@Radomir-Drukh The code example above has accessibility issues. The extra column and the links inside it are displayed in the accessibility tree without any description. And these links don't have any focus styles.

Is there any similar functionality in other libraries? If so, how is it implemented?

And it would be nice to see a working example for @gravity-ui/table.

It seems this can be easily done with getRowModel.

@Raubzeug Could you give more details on how getRowModel can help here?

Raubzeug commented 1 month ago

@beliarh my bad, it seem no help... I've thought about any instrument, that will allow to render a fake transparent link element, that will stretch above row and will allow user to interact with row like with native link.