feat(SubBlocks): add possibility to position controls within cards at footer #823

DC-RomanKarpov · 2024-02-20T12:12:15Z

No description provided.

gravity-ui-bot · 2024-02-20T12:17:37Z

gorgeousvlad · 2024-02-21T08:23:33Z

src/components/index.ts

@@ -6,6 +6,7 @@ export {default as BackLink} from './BackLink/BackLink';
 export {default as BalancedMasonry} from './BalancedMasonry/BalancedMasonry';
 export {default as BlockBase} from './BlockBase/BlockBase';
 export {default as Button} from './Button/Button';
+export {Buttons} from './Buttons/Buttons';


could we make it consistent with other exports?

gorgeousvlad · 2024-02-21T08:25:45Z

src/sub-blocks/BackgroundCard/BackgroundCard.tsx

                    colSizes={{all: 12, md: 12}}
                />
            </CardBase.Content>
+            <CardBase.Footer className={b('footer')}>


{ controlPosition === 'footer' && <CardBase.Footer className={b('footer')}>...

That does not work that way =)
The CardBase component requires that children type will ReactElement | ReactElement[] I thought about separate child creation but it looks quite ugly.

gorgeousvlad · 2024-02-21T08:30:24Z

src/models/constructor-items/sub-blocks.ts

@@ -140,6 +140,7 @@ export interface BackgroundCardProps
    background?: ThemeSupporting<ImageObjectProps>;
    paddingBottom?: 's' | 'm' | 'l' | 'xl';
    backgroundColor?: string;
+    controlPosition?: 'content' | 'footer';
 }

 export interface BasicCardProps


`interface CardLayoutProps = {
controlPosition?: 'content' | 'footer';
}

...

export interface BannerCardProps extends CardLayoutProps
`

gorgeousvlad · 2024-02-21T08:31:59Z

src/sub-blocks/BackgroundCard/BackgroundCard.tsx

@@ -46,16 +51,30 @@ const BackgroundCard = (props: BackgroundCardProps) => {
                    style={{backgroundColor}}
                />
                <Content
+                    titleId={titleId}


const controlPositionFooter = controlPosition === 'footer'
...
links={controlPositionFooter ? undefined : links}

gorgeousvlad · 2024-02-21T08:33:26Z

src/sub-blocks/BasicCard/BasicCard.tsx

                        size="s"
                        colSizes={{all: 12, md: 12}}
                    />
                </IconWrapper>
            </CardBase.Content>
+            <CardBase.Footer className={b('footer')}>


confused a littler that we should copy-paster this 3 times, could we create component for this?

The main problem that no, we can't move it to a separate component without CardBase refactoring. The component checks types of its children. However we can try to create a helper util... But I'm not sure that will be more readable or more convenient in future.

gorgeousvlad · 2024-02-28T16:19:35Z

src/sub-blocks/BasicCard/BasicCard.tsx

                        size="s"
                        colSizes={{all: 12, md: 12}}
                    />
                </IconWrapper>
            </CardBase.Content>
+            {areControlsInFooter && (links || buttons) && (


this code repeats 3 times in pr, could we create kinda component?

As I wrote earlier, it's impossible to wrap CardBase.Footer in other components. Because the CardBase component uses its children's types within itself. Therefore I think that we should not update code here.

Wrote helper functions to implement the required changes

gorgeousvlad · 2024-02-28T16:22:23Z

src/components/CardBase/CardBase.tsx

+    HTMLAttributeAnchorTarget,
+    PropsWithChildren,
+    ReactElement,
+    isValidElement,


what is the purpose of isValidElement? didn't find any usage examples in pr

Please have a look at line 96. It used to handle children elements correctly.

gorgeousvlad · 2024-02-28T16:26:45Z

src/sub-blocks/BackgroundCard/__stories__/BackgroundCard.stories.tsx

+            <BlockBase>
+                <CardLayout
+                    title="With controlPosition = footer"
+                    description="Please note that the controlPosition prop manages the position of buttons and links only when paddingBottom = default."


We don't store texts in .tsx story files, but keep them in data.json. In case we want to translate stories it would be a problem to pick all of the texts form code

gorgeousvlad · 2024-02-28T16:27:59Z

src/sub-blocks/BasicCard/BasicCard.tsx


    return (
        <CardBase
            className={b()}
            {...cardParams}
            extraProps={{'aria-describedby': descriptionId, 'aria-labelledby': titleId}}
        >
-            <CardBase.Content>
+            <CardBase.Content key="content">


why do we need to add a constant key?

There is no necessity. Will remove.
In the process of development, before CardBase refactoring, it must be added to a child because I passed CardBase children as array of elements. Thanks for the notice.

gorgeousvlad · 2024-02-28T16:28:29Z

src/sub-blocks/BasicCard/__stories__/BasicCard.stories.tsx

@@ -135,3 +171,19 @@ WithUrl.args = {
 WithContentList.args = {
    ...DefaultArgs,
 } as BasicCardProps;
+
+ControlPosition.argTypes = {


is it necessary?

We can omit controls' hiding, but I think that these controls are unnecessary.

… footer

gorgeousvlad · 2024-02-29T21:16:06Z

src/utils/renderContentControls/renderContentControls.tsx

+    size?: ContentSize;
+    qa?: Record<string, string>;
+};
+const renderContentControls = (


type ContentControlsProps = { links?: LinkProps[]; buttons?: ButtonProps[]; titleId?: string; size?: ContentSize; qa?: Record<string, string>; children: (children: React.ReactElement) => React.ReactElement; }; export const ContentControls = (props: ContentControls) => { const {links, buttons, titleId, size = 's', qa = {}, children} = props; const {links: linksQa, link: linkQa, buttons: buttonsQa, button: buttonQa} = qa; return links || buttons ? children( <Fragment> <Links className={b('links', {size})} size={size} links={links} titleId={titleId} qa={linksQa} linkQa={linkQa} /> <Buttons className={b('buttons', {size})} size={size} buttons={buttons} titleId={titleId} qa={buttonsQa} buttonQa={buttonQa} /> </Fragment>, ) : null; }

The suggested code would be treated as a React Component. Thus it won't work within CardBase. Please, have a look at

page-constructor/src/components/CardBase/CardBase.tsx

Line 79 in 71ca3d9

switch (child.type) {

gorgeousvlad · 2024-02-29T21:20:42Z

src/sub-blocks/BackgroundCard/BackgroundCard.tsx

                    colSizes={{all: 12, md: 12}}
                />
            </CardBase.Content>
+            {footerControls}


<ContentControls ...> {(children) => renderCardFooterControlsContainer(children)} </ContentControls>

Unfortunately, this won't work within CardBase.

DC-RomanKarpov self-assigned this Feb 20, 2024

DC-RomanKarpov requested review from gorgeousvlad and NikitaCG as code owners February 20, 2024 12:12

DC-RomanKarpov changed the title ~~feat(SubBlocks): add possibility to position controls within cards at…~~ feat(SubBlocks): add possibility to position controls within cards at footer Feb 20, 2024

gorgeousvlad requested changes Feb 21, 2024

View reviewed changes

DC-RomanKarpov linked an issue Feb 22, 2024 that may be closed by this pull request

Add possibility to place card controls (links & buttons) in a card footer #826

Closed

DC-RomanKarpov force-pushed the feat-option-to-place-card-controls-in-footer branch from f074402 to 75c36fd Compare February 27, 2024 12:11

DC-RomanKarpov requested a review from gorgeousvlad February 27, 2024 20:25

gorgeousvlad requested changes Feb 28, 2024

View reviewed changes

DC-RomanKarpov added 5 commits February 28, 2024 22:17

feat(SubBlocks): add possibility to position controls within cards at…

e6ed9c3

… footer

fix: comments

cec49fa

feat: add storybook for changes

ff0e912

fix: apply the required behavioral changes

af1e814

fix: resolve comments

09dad73

DC-RomanKarpov force-pushed the feat-option-to-place-card-controls-in-footer branch from ddcbe4c to 09dad73 Compare February 28, 2024 21:17

fix: lint

71ca3d9

DC-RomanKarpov requested a review from gorgeousvlad February 29, 2024 12:39

gorgeousvlad approved these changes Feb 29, 2024

View reviewed changes

DC-RomanKarpov merged commit 7e3dc0b into main Mar 1, 2024
3 checks passed

DC-RomanKarpov deleted the feat-option-to-place-card-controls-in-footer branch March 1, 2024 08:34

gravity-ui-bot mentioned this pull request Mar 1, 2024

chore(main): release 4.56.0 #839

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(SubBlocks): add possibility to position controls within cards at footer #823

feat(SubBlocks): add possibility to position controls within cards at footer #823

DC-RomanKarpov commented Feb 20, 2024 •

edited

Loading

gravity-ui-bot commented Feb 20, 2024

gorgeousvlad Feb 21, 2024

gorgeousvlad Feb 21, 2024

DC-RomanKarpov Feb 21, 2024

gorgeousvlad Feb 21, 2024

gorgeousvlad Feb 21, 2024

gorgeousvlad Feb 21, 2024

DC-RomanKarpov Feb 21, 2024

gorgeousvlad Feb 28, 2024

DC-RomanKarpov Feb 28, 2024 •

edited

Loading

gorgeousvlad Feb 28, 2024

DC-RomanKarpov Feb 28, 2024

gorgeousvlad Feb 28, 2024

gorgeousvlad Feb 28, 2024

DC-RomanKarpov Feb 28, 2024

gorgeousvlad Feb 28, 2024

DC-RomanKarpov Feb 28, 2024

gorgeousvlad Feb 29, 2024

DC-RomanKarpov Mar 1, 2024

gorgeousvlad Feb 29, 2024

DC-RomanKarpov Mar 1, 2024

feat(SubBlocks): add possibility to position controls within cards at footer #823

feat(SubBlocks): add possibility to position controls within cards at footer #823

Conversation

DC-RomanKarpov commented Feb 20, 2024 • edited Loading

gravity-ui-bot commented Feb 20, 2024

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 Feb 28, 2024 • 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

DC-RomanKarpov commented Feb 20, 2024 •

edited

Loading

DC-RomanKarpov Feb 28, 2024 •

edited

Loading