Skip to content
This repository has been archived by the owner on Jul 28, 2024. It is now read-only.

[FRNT-485]: Generate map of component states #122

Merged
merged 1 commit into from
Jun 21, 2021

Conversation

tatinacher
Copy link
Member

Generate map of component states

  • FRNT-485

@tatinacher tatinacher marked this pull request as draft May 25, 2021 15:41
@github-actions github-actions bot added the @atoms Changes inside atoms label May 25, 2021
@tatinacher tatinacher force-pushed the feat/FRNT-485-generate-map-of-states branch 2 times, most recently from b4cf180 to 741bf1e Compare May 26, 2021 10:46
@tatinacher tatinacher marked this pull request as ready for review May 26, 2021 10:47
sergeysova
sergeysova previously approved these changes Jun 7, 2021
Copy link
Contributor

@ainursharaev ainursharaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В целом хорошо, нужно исправить в паре мест

package.json Outdated
@@ -49,6 +49,7 @@
"@typescript-eslint/parser": "4.15.0",
"@wessberg/rollup-plugin-ts": "^1.3.8",
"babel-loader": "^8.2.2",
"babel-plugin-extract-react-types": "^0.1.14",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А для чего добавила этот плагин?

interface ButtonIconProps {
disabled: boolean;
filled: boolean;
sizes: 'N' | 'XS' | 'S' | 'M' | 'L' | 'XL' | 'H';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

может экспортнуть тип из lib/map-generation?

};

export const GenerateButtonIconMap = () => (
<GenerateMap component={ComponentButtonIcon} x={variants} y={sizes} otherProps={props} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

немного путают пропсы, так как x ближе к группам вариаций, а otherProps больше походит на x ось внутри группы


const deepCopy = (obj: Array<Record<string, unknown>>) => JSON.parse(JSON.stringify(obj));

export const showHead = (props: Record<string, unknown>) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Название сбивает столку. Функция формирует заголовок группы из пропсов, может как-нибудь отразить это в названии?

{variants.map((variant, i) => (
<VariantBlock key={i}>
<Header>{variant.name}</Header>
<GridTemplate columns={variant.columns + 1}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше убрать вычисление значения columns внутрь generateMap

Comment on lines 47 to 50
{variant.values[0].values.map(({ variants, sizes, ...prop }, headKey) => (
<div key={`head-${headKey}`}>{showHead(prop)}</div>
))}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

раз эти комбианции повторяются, можно вынести, к примеру, отдельным свойством на уровне корня variant

import * as React from 'react';
import { IconSearch } from 'static/icons';

import { GenerateMap, SizeBlock } from '../../../lib/map-generation';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

импорт из lib/map-generation

import { ButtonIcon, Chip } from 'ui';
import { IconSearch } from 'static/icons';

import { GenerateMap, SizeBlock } from '../../../lib/map-generation';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

импорт из lib/map-generation

import { IconSearch } from 'static/icons';

import { Button } from '.';
import { GenerateMap, SizeBlock } from '../../../lib/map-generation';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

импорт из lib/map-generation

import { ButtonIcon } from 'ui';
import { IconSearch } from 'static/icons';

import { GenerateMap, SizeBlock } from '../../../lib/map-generation';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

импорт из lib/map-generation

Copy link
Member

@sergeysova sergeysova Jun 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@purplecandle

Suggested change
import { GenerateMap, SizeBlock } from '../../../lib/map-generation';
import { GenerateMap, SizeBlock } from 'lib/map-generation';

Можно сразу подкинуть code suggestion, чтобы было проще

@tatinacher tatinacher force-pushed the feat/FRNT-485-generate-map-of-states branch from e216de8 to 065120c Compare June 18, 2021 15:57
GenerateMapProps & { component: React.ComponentType<unknown> }
> = ({ sizes, variants, otherProps, component }) => {
const Component = component;
const variantsMap = generateMap({ y: sizes, x: variants, otherProps });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а что прилетает в otherProps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

любые пропсы состояний компонента

) => {
let tree = [main];

Object.keys(otherProps).forEach((key: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а через reduce тут разве нельзя?

@sergeysova sergeysova merged commit e872977 into master Jun 21, 2021
@sergeysova sergeysova deleted the feat/FRNT-485-generate-map-of-states branch June 21, 2021 09:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@atoms Changes inside atoms
Projects
None yet
4 participants