Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: long headers values #76

Merged
merged 15 commits into from
Aug 14, 2023
Merged

fix: long headers values #76

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8eb531f
fix: long headers values
Aug 8, 2023
7af5521
fix: add long title popover to accordeon & fix placement usage
Aug 9, 2023
c229b1f
fix: add long title popover to card & section
Aug 9, 2023
64db05e
fix: constants
Aug 9, 2023
807997e
fix: review updates
Aug 10, 2023
bf43df5
Merge branch 'main' into fix_headers
saxumcordis Aug 10, 2023
ec3b257
fix: long headers values
Aug 8, 2023
e035e1e
fix: add long title popover to accordeon & fix placement usage
Aug 9, 2023
737b8c3
fix: add long title popover to card & section
Aug 9, 2023
3e2d318
fix: constants
Aug 9, 2023
3dfa2cd
fix: review updates
Aug 10, 2023
180516e
fix: fixed array type for default values (#72)
NasgulNexus Aug 8, 2023
183c98c
Merge branch 'fix_headers' of https://github.com/gravity-ui/dynamic-f…
Aug 10, 2023
3c6c2de
fix: imports
Aug 11, 2023
7b7056f
Merge branch 'main' into fix_headers
saxumcordis Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/lib/kit/components/Card/Card.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@
font-size: var(--yc-text-subheader-2-font-size);
font-weight: var(--yc-text-subheader-font-weight);
line-height: var(--yc-text-subheader-2-line-height);
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
max-width: 533px;
}

&__note {
Expand Down
20 changes: 15 additions & 5 deletions src/lib/kit/components/Card/Card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import {HelpPopover} from '@gravity-ui/components';
import {ChevronDown} from '@gravity-ui/icons';
import {Button, Card as CardBase, Icon} from '@gravity-ui/uikit';
import {Button, Card as CardBase, Icon, Popover, type PopoverProps} from '@gravity-ui/uikit';
import _ from 'lodash';

import {block} from '../../utils';
Expand All @@ -24,6 +24,9 @@ export interface CardProps {
checkEmptyBody?: boolean;
}

const POPOVER_PLACEMENT: PopoverProps['placement'] = ['bottom', 'top'];
Copy link
Contributor

Choose a reason for hiding this comment

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

there is a separate file of constants you need to take out there

const MAX_TITLE_WIDTH = 533;

export const Card: React.FC<CardProps> = ({
name,
title: propsTitle,
Expand All @@ -37,6 +40,7 @@ export const Card: React.FC<CardProps> = ({
children,
}) => {
const containerRef = React.useRef<HTMLDivElement>(null);
const titleRef = React.useRef<HTMLDivElement>(null);
const bodyRef = React.useRef<HTMLDivElement>(null);
const [open, setOpen] = React.useState(alwaysOpen || propsOpen || false);

Expand Down Expand Up @@ -66,22 +70,26 @@ export const Card: React.FC<CardProps> = ({
[],
);

const titlePopoverDisabled = (titleRef.current?.offsetWidth || 0) < MAX_TITLE_WIDTH;
Copy link
Contributor

Choose a reason for hiding this comment

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

const titlePopoverDisabled =
            (this.titleRef.current?.offsetWidth || 0) <= MAX_TITLE_TEXT_WIDTH;


const title = React.useMemo(() => {
if (_.isString(propsTitle)) {
return (
<React.Fragment>
<div className={b('title')}>{propsTitle}</div>
<Popover content={propsTitle} disabled={titlePopoverDisabled}>
<div className={b('title')}>{propsTitle}</div>
</Popover>
{description ? (
<div className={b('note')}>
<HelpPopover htmlContent={description} placement={['bottom', 'top']} />
<HelpPopover htmlContent={description} placement={POPOVER_PLACEMENT} />
</div>
) : null}
</React.Fragment>
);
}

return propsTitle;
}, [propsTitle, description]);
}, [propsTitle, titlePopoverDisabled, description]);

React.useEffect(() => {
if (!alwaysOpen && propsOpen !== undefined && propsOpen !== open) {
Expand Down Expand Up @@ -112,7 +120,9 @@ export const Card: React.FC<CardProps> = ({
className={b('header', {interactive: Boolean(handleHeaderToggle)})}
onClick={handleHeaderToggle}
>
<div className={b('header-left')}>{title}</div>
<div ref={titleRef} className={b('header-left')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

ref should be on another div
right here
<div className={b('title')}>{propsTitle}</div>
otherwise it will be like this
image

{title}
</div>
<div className={b('header-right')} onClick={preventEvent}>
{actions ? <div className={b('actions')}>{actions}</div> : null}
{alwaysOpen ? null : (
Expand Down
4 changes: 4 additions & 0 deletions src/lib/kit/components/Layouts/Section/Section.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ $block: '.#{$ns}section';
}

&__title {
max-width: 533px;
font-weight: 500;
margin: 0;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;

&_size_s {
font-size: 13px;
Expand Down
32 changes: 24 additions & 8 deletions src/lib/kit/components/Layouts/Section/Section.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import React from 'react';

import {HelpPopover} from '@gravity-ui/components';
import {Popover, type PopoverProps} from '@gravity-ui/uikit';

import {GroupIndent} from '../../';
import {ErrorWrapper} from '../../../';
import {
FieldRenderProps,
FieldValue,
FormValue,
LayoutProps,
Spec,
ViewLayoutProps,
type FieldRenderProps,
Copy link
Contributor

Choose a reason for hiding this comment

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

do not change code that is not related to the current task

type FieldValue,
type FormValue,
type LayoutProps,
type Spec,
type ViewLayoutProps,
isArraySpec,
isObjectSpec,
} from '../../../../core';
Expand All @@ -27,6 +28,9 @@ interface SectionProps {
descriptionAsSubtitle?: boolean;
}

const POPOVER_PLACEMENT: PopoverProps['placement'] = ['bottom', 'top'];
const MAX_TITLE_WIDTH = 533;
Copy link
Contributor

Choose a reason for hiding this comment

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

move to common constants


const SectionBase = <D extends FieldValue, T extends FormValue, S extends Spec>({
name,
spec,
Expand All @@ -39,6 +43,7 @@ const SectionBase = <D extends FieldValue, T extends FormValue, S extends Spec>(
}: (LayoutProps<D, S> | ViewLayoutProps<T, S>) & SectionProps) => {
const meta = (restProps as FieldRenderProps<D>).meta as FieldRenderProps<D>['meta'] | undefined;
const arrOrObjFlag = isArraySpec(spec) || isObjectSpec(spec);
const titleRef = React.useRef<HTMLHeadingElement>(null);
let content = children;

if (meta) {
Expand Down Expand Up @@ -76,15 +81,26 @@ const SectionBase = <D extends FieldValue, T extends FormValue, S extends Spec>(
}
}

const layoutTitle = spec.viewSpec.layoutTitle;
const layoutTitlePopoverDisabled = (titleRef.current?.offsetWidth || 0) < MAX_TITLE_WIDTH;
Copy link
Contributor

Choose a reason for hiding this comment

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

const layoutTitlePopoverDisabled = (titleRef.current?.offsetWidth || 0) <= MAX_TITLE_WIDTH;
image


return (
<section className={b()}>
{spec.viewSpec.layoutTitle ? (
{layoutTitle ? (
<div
className={b('header', {
'with-popover': !descriptionAsSubtitle,
})}
>
<h2 className={b('title', {size: titleSize})}>{spec.viewSpec.layoutTitle}</h2>
<Popover
content={layoutTitle}
placement={POPOVER_PLACEMENT}
disabled={layoutTitlePopoverDisabled}
>
<h2 className={b('title', {size: titleSize})} ref={titleRef}>
{layoutTitle}
</h2>
</Popover>
{description}
</div>
) : null}
Expand Down
1 change: 1 addition & 0 deletions src/lib/kit/components/SimpleVerticalAccordeon/SimpleVerticalAccordeon.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ $animationDuration: 0.3s;

&-inner {
margin-left: -13px;
max-width: 543px;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is there 543 here while everywhere is 533?

}
}

Expand Down
41 changes: 32 additions & 9 deletions src/lib/kit/components/SimpleVerticalAccordeon/SimpleVerticalAccordeon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import {HelpPopover} from '@gravity-ui/components';
import {ChevronDown} from '@gravity-ui/icons';
import {Button, Icon} from '@gravity-ui/uikit';
import {Button, Icon, Popover, type PopoverProps} from '@gravity-ui/uikit';

import {block} from '../../utils';

Expand Down Expand Up @@ -34,6 +34,9 @@ interface SimpleVerticalAccordeonState {
isFirstRender: boolean;
}

const MAX_TITLE_TEXT_WIDTH = 495; /** 543px max-width - 48px of padding */
const POPOVER_PLACEMENT: PopoverProps['placement'] = ['bottom', 'top'];

export class SimpleVerticalAccordeon extends React.Component<
SimpleVerticalAccordeonProps,
SimpleVerticalAccordeonState
Expand All @@ -45,6 +48,7 @@ export class SimpleVerticalAccordeon extends React.Component<
};

componentRef = React.createRef<HTMLDivElement>();
titleRef = React.createRef<HTMLElement>();

constructor(props: SimpleVerticalAccordeonProps) {
super(props);
Expand Down Expand Up @@ -97,19 +101,38 @@ export class SimpleVerticalAccordeon extends React.Component<
return null;
}

const title = this.getTitle();
const titlePopoverDisabled =
(this.titleRef.current?.offsetWidth || 0) < MAX_TITLE_TEXT_WIDTH;
Copy link
Contributor

Choose a reason for hiding this comment

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

const titlePopoverDisabled = (this.titleRef.current?.offsetWidth || 0) <= MAX_TITLE_TEXT_WIDTH;


return (
Boolean(React.Children.count(children)) && (
<div className={b({branch: withBranchView, view: viewLayout}, className)}>
<div className={b('header')}>
<Button
view="flat"
className={b('header-inner', buttonClassName)}
onClick={this.handleClick}
qa={`${name}-accordeon-toggler`}
<Popover
content={title}
disabled={titlePopoverDisabled}
placement={POPOVER_PLACEMENT}
>
<b className={b('title', {size: titleSize})}>{this.getTitle()}</b>
<Icon data={ChevronDown} className={b('chevron', {open})} size={16} />
</Button>
<Button
view="flat"
className={b('header-inner', buttonClassName)}
onClick={this.handleClick}
qa={`${name}-accordeon-toggler`}
width="auto"
>
{' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the extra space

<b ref={this.titleRef} className={b('title', {size: titleSize})}>
{title}
</b>
<Icon
data={ChevronDown}
className={b('chevron', {open})}
size={16}
/>
</Button>
</Popover>

{this.getTooltip()}
{headerActionsTemplate ? headerActionsTemplate : null}
</div>
Expand Down
6 changes: 4 additions & 2 deletions src/lib/kit/components/Views/MultiSelectView/MultiSelectView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';

import {Popover} from '@gravity-ui/uikit';
import {Popover, type PopoverProps} from '@gravity-ui/uikit';

import {ArrayView} from '../../../../core';
import {block} from '../../../utils';
Expand All @@ -9,6 +9,8 @@ import './MultiSelectView.scss';

const b = block('multiselect-view');

const POPOVER_PLACEMENT: PopoverProps['placement'] = ['bottom', 'top'];
Copy link
Contributor

Choose a reason for hiding this comment

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

these changes do not apply to the current task, you should remove them, if you want to change imports and parameters, it is better to do it in a separate task


export const MultiSelectView: ArrayView = ({spec, value = []}) => {
const _value = value as string[];

Expand All @@ -21,7 +23,7 @@ export const MultiSelectView: ArrayView = ({spec, value = []}) => {
<React.Fragment>
{items.map((item) => (
<Popover
placement={['bottom', 'top']}
placement={POPOVER_PLACEMENT}
key={item}
content={item}
className={b('tooltip-container')}
Expand Down
6 changes: 4 additions & 2 deletions src/lib/kit/components/Views/NumberWithScaleView/NumberWithScaleView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';

import {Popover} from '@gravity-ui/uikit';
import {Popover, type PopoverProps} from '@gravity-ui/uikit';

import {StringViewProps} from '../../../../core';
import {block, isCorrectSizeParams} from '../../../utils';
Expand All @@ -10,14 +10,16 @@ import './NumberWithScaleView.scss';

const b = block('number-with-scale-view');

const POPOVER_PLACEMENT: PopoverProps['placement'] = ['bottom', 'top'];
Copy link
Contributor

Choose a reason for hiding this comment

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

these changes do not apply to the current task, you should remove them, if you want to change imports and parameters, it is better to do it in a separate task


const NumberWithScaleViewBase: React.FC<StringViewProps> = ({spec, value}) => {
const {initialValue, initialType} = useInitial(value || '', spec, [value]);
const {scale} = spec.viewSpec.sizeParams!;

return (
<div className={b()}>
<Popover
placement={['bottom', 'top']}
placement={POPOVER_PLACEMENT}
content={initialValue}
className={b('tooltip-container')}
contentClassName={b('tooltip')}
Expand Down