feat(FilterBlock): add block that filters its subblocks #180

DC-RomanKarpov · 2023-02-28T09:52:06Z

No description provided.

gravity-ui-bot · 2023-02-28T09:54:33Z

Preview is ready.

src/blocks/CardLayout/CardLayout.tsx

src/blocks/CardLayout/withCardLayoutItem.tsx

src/blocks/FilterBlock/FilterBlock.tsx

src/blocks/FilterBlock/README.md

src/blocks/FilterBlock/__stories__/data.json

src/components/ButtonTabs/ButtonTabs.scss

src/containers/PageConstructor/components/ConstructorBlocks/ConstructorBlocks.tsx

src/context/innerContext/InnerContext.ts

yuberdysheva · 2023-02-28T10:43:58Z

Typecheck failed

src/blocks/FilterBlock/FilterBlock.tsx

gorgeousvlad · 2023-03-02T12:26:49Z

src/blocks/FilterBlock/FilterBlock.scss

+    &__title {
+        margin-bottom: $indentSM;
+
+        @include centerableTitle();


we are trying to use kebab-notation for css mixins

gorgeousvlad · 2023-03-02T12:27:49Z

src/blocks/FilterBlock/FilterBlock.tsx

+import './FilterBlock.scss';
+
+const b = block('filter-block');
+const DEFAULT_ALL_TAG_TITLE = 'All';


need to use i18n with text strings like this

gorgeousvlad · 2023-03-02T12:30:20Z

src/blocks/FilterBlock/FilterBlock.tsx

+            : undefined;
+        const otherButtons: ButtonTabsItemProps[] | undefined =
+            filterTags && filterTags.map((tag) => ({id: tag.id, title: tag.label}));
+        return [...(allButton ? [allButton] : []), ...(otherButtons ? otherButtons : [])];


allButton ?? : []

Unfortunately cannot change this line as suggested, because we merge here two arrays to omit nulls in final array. Replacing ternary operator with the ?? operator because in this case type of result wont be an array.

gorgeousvlad · 2023-03-02T12:32:49Z

src/blocks/FilterBlock/FilterBlock.tsx

+        return [...(allButton ? [allButton] : []), ...(otherButtons ? otherButtons : [])];
+    }, [allTag, filterTags]);
+
+    const [selectedTag, setSelectedTag] = useState(tabButtons.length ? tabButtons[0].id : null);


can we return null if there is no tabButtons to avoid checking it's length below?

Yes we can, but it will make the body of the memo more complex, I believe it's better to leave it as is.

gorgeousvlad · 2023-03-02T12:36:29Z

src/blocks/FilterBlock/FilterBlock.tsx

+
+    const [selectedTag, setSelectedTag] = useState(tabButtons.length ? tabButtons[0].id : null);
+
+    const actualTag: string | null = useMemo(() => {


Didn't catch why do we need this variable? I think selectedTag is enough, isn't it?

Let's assume that we have 3 ids, 'A', 'B', 'C'. ` We change the value of selected tag to 'C', then remove 'C' from our id list. In this case we should choose a new id to display. The actualTag is this new selected value, or the selectedTag if it still exists.

But how can we reproduce this situation? We don't have multiple tags choice or an opportunity to let no tags be selected

gorgeousvlad · 2023-03-02T13:03:47Z

src/models/constructor-items/blocks.ts

+export interface FilterBlockProps extends Animatable, LoadableChildren {
+    title?: TitleProps | string;
+    description?: string;
+    filterTags: FilterTag[];


could it be just tags? filterTags sounds like function name

gorgeousvlad · 2023-03-02T13:04:14Z

src/models/constructor-items/blocks.ts

+    description?: string;
+    filterTags: FilterTag[];
+    items: FilterItem[];
+    tagSize?: ButtonSize;


There is no entity named tabs in the block. Maybe buttonSize?

tagButtonSize =)

gorgeousvlad · 2023-03-02T13:05:30Z

src/containers/PageConstructor/PageConstructor.tsx

-                blockTypes: [...BlockTypes, ...getCustomBlockTypes(props.custom)],
+                blockTypes: [...BlockTypes, ...getCustomBlockTypes(custom)],
+                subBlockTypes: [...SubBlockTypes, ...getCustomSubBlockTypes(custom)],
+                headerBlockTypes: [...HeaderBlockTypes, ...getCustomHeaderTypes(custom)],


why do we need subBlockTypes and headerBlockTypes in context?

consistency of props of the context and return values of the memo. Avoid returning of same instances differently. In context and outside it.

gorgeousvlad · 2023-03-02T13:07:07Z

src/models/constructor-items/blocks.ts

@@ -32,7 +32,8 @@ import {
 } from './common';
 import {ThemeSupporting} from '../../utils';
 import {GridColumnSize, GridColumnSizesType} from '../../grid/types';
-import {BannerCardProps, SubBlock} from './sub-blocks';
+import {BannerCardProps, SubBlock, SubBlockModels} from './sub-blocks';
+import {ButtonSize} from '@gravity-ui/uikit';


upper than internal imports

gorgeousvlad · 2023-03-02T13:13:04Z

src/models/constructor-items/common.ts

@@ -178,6 +178,7 @@ export interface ButtonProps {
    metrikaGoals?: MetrikaGoal;
    pixelEvents?: ButtonPixel;
    target?: string;
+    selected?: boolean;


Why do we need to add this?

Sorry, did not removed after testing one possible solution. Done.

gorgeousvlad · 2023-03-06T16:37:04Z

src/blocks/FilterBlock/FilterBlock.tsx

+
+    const [selectedTag, setSelectedTag] = useState(tabButtons.length ? tabButtons[0].id : null);
+
+    const actualTag: string | null = useMemo(() => {


But how can we reproduce this situation? We don't have multiple tags choice or an opportunity to let no tags be selected

gorgeousvlad · 2023-03-06T16:45:09Z

src/models/constructor-items/blocks.ts

+    description?: string;
+    filterTags: FilterTag[];
+    items: FilterItem[];
+    tagSize?: ButtonSize;


tagButtonSize =)

DC-RomanKarpov self-assigned this Feb 28, 2023

DC-RomanKarpov requested review from gorgeousvlad and yuberdysheva as code owners February 28, 2023 09:52

yuberdysheva suggested changes Feb 28, 2023

View reviewed changes

DC-RomanKarpov requested a review from yuberdysheva February 28, 2023 12:23

DC-RomanKarpov force-pushed the roman-karpov/filter-block branch from 9bab2fd to a3b752e Compare February 28, 2023 15:25

yuberdysheva suggested changes Mar 1, 2023

View reviewed changes

src/blocks/FilterBlock/FilterBlock.tsx Outdated Show resolved Hide resolved

yuberdysheva approved these changes Mar 1, 2023

View reviewed changes

gorgeousvlad suggested changes Mar 2, 2023

View reviewed changes

DC-RomanKarpov requested a review from gorgeousvlad March 2, 2023 14:49

gorgeousvlad approved these changes Mar 6, 2023

View reviewed changes

DC-RomanKarpov added 12 commits March 6, 2023 18:07

feat(FilterBlock): add block that filters its child's subblocks

56468c8

fix: comments and check errors

be5903e

fix: remove unnecessary newlines

f563b89

fix: fix rest comments

3cb7f89

fix: lint

772267b

fix: make all tag null

1e6b097

fix: simplify the FilterBlock to show only cards

05127f7

feat: make header and tab panel centerable

23782e8

fix: resolve comments

17c7384

fix: move button mixin to shared lib

2803aaf

fix: fix merge

428b03d

fix: rename tagSize to tagButtonSize

b570dce

DC-RomanKarpov force-pushed the roman-karpov/filter-block branch from 5e69b84 to b570dce Compare March 6, 2023 17:17

DC-RomanKarpov merged commit 4b6852c into main Mar 6, 2023

DC-RomanKarpov deleted the roman-karpov/filter-block branch March 6, 2023 18:08

gravity-ui-bot mentioned this pull request Mar 6, 2023

chore(main): release 1.21.0 #204

Merged

DC-RomanKarpov changed the title ~~feat(FilterBlock): add block that filters its child's subblocks~~ feat(FilterBlock): add block that filters its subblocks Mar 6, 2023

gravity-ui-bot mentioned this pull request Sep 19, 2023

chore(version-1.x.x/fixes): release 1.27.2 #574

Merged

NikitaCG mentioned this pull request Mar 6, 2024

chore(main): release 6.0.0 gravity-ui/blog-constructor#186

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(FilterBlock): add block that filters its subblocks #180

feat(FilterBlock): add block that filters its subblocks #180

DC-RomanKarpov commented Feb 28, 2023

gravity-ui-bot commented Feb 28, 2023

yuberdysheva commented Feb 28, 2023 •

edited

Loading

gorgeousvlad Mar 2, 2023

gorgeousvlad Mar 2, 2023

gorgeousvlad Mar 2, 2023

DC-RomanKarpov Mar 2, 2023

gorgeousvlad Mar 2, 2023

DC-RomanKarpov Mar 2, 2023

gorgeousvlad Mar 2, 2023

DC-RomanKarpov Mar 2, 2023

gorgeousvlad Mar 6, 2023

gorgeousvlad Mar 2, 2023

gorgeousvlad Mar 2, 2023

DC-RomanKarpov Mar 2, 2023

gorgeousvlad Mar 6, 2023

gorgeousvlad Mar 2, 2023

DC-RomanKarpov Mar 2, 2023 •

edited

Loading

gorgeousvlad Mar 2, 2023

gorgeousvlad Mar 2, 2023

DC-RomanKarpov Mar 2, 2023

gorgeousvlad Mar 6, 2023

gorgeousvlad Mar 6, 2023


		const [selectedTag, setSelectedTag] = useState(tabButtons.length ? tabButtons[0].id : null);

		const actualTag: string \| null = useMemo(() => {

feat(FilterBlock): add block that filters its subblocks #180

feat(FilterBlock): add block that filters its subblocks #180

Conversation

DC-RomanKarpov commented Feb 28, 2023

gravity-ui-bot commented Feb 28, 2023

yuberdysheva commented Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DC-RomanKarpov Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuberdysheva commented Feb 28, 2023 •

edited

Loading

DC-RomanKarpov Mar 2, 2023 •

edited

Loading