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

Refactor header using GravityUI #566

Merged
merged 3 commits into from
Jul 12, 2024
Merged

Refactor header using GravityUI #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c398a0e
refactor: refactor header using GravityUI
Kabir-Ivan Jul 2, 2024
b037f25
refactor: minor changes
Kabir-Ivan Jul 10, 2024
abad3fc
refactor: fix browser select
Kabir-Ivan Jul 10, 2024
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
10 changes: 6 additions & 4 deletions lib/static/components/controls/base-host-input.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React, {Component} from 'react';
import PropTypes from 'prop-types';
import {bindActionCreators} from 'redux';
import {connect} from 'react-redux';
import {TextInput} from '@gravity-ui/uikit';
import * as actions from '../../modules/actions';

class BaseHostInput extends Component {
Expand All @@ -19,12 +20,13 @@ class BaseHostInput extends Component {

render() {
return (
<input
className="control-input basehost__text"
<TextInput
size='m'
className='base-host-input'
value={this.props.baseHost}
placeholder="change original host for view in browser"
placeholder="Base host for view in browser"
onChange={this._onChange}
data-test-id='base-host'
qa='base-host'
/>
);
}
Expand Down
325 changes: 135 additions & 190 deletions lib/static/components/controls/browser-list/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,224 +5,169 @@ import {flatten, isEmpty, get, chain, compact} from 'lodash';
import PropTypes from 'prop-types';
import CheckboxTree from 'react-checkbox-tree';

import Label from './label';
import {mkBrowserIcon, buildComplexId} from './utils';
import Popup from '../../popup';
import ArrayContainer from '../../../containers/array';

import 'react-checkbox-tree/lib/react-checkbox-tree.css';
import './index.styl';

const BrowserList = (props) => {
const {available, onChange, selected: selectedProp} = props;

const [expanded, setExpanded] = useState([]);
const [selected, setSelected] = useState(buidSelectedItems());
const [elements, setElements] = useState([]);
const treeDataMap = useMemo(buildTreeDataMap, [available]);
const nodes = useMemo(prepareTreeData, [available]);
const selectAll = useCallback(_selectAll, [setExpanded, treeDataMap]);

useEffect(() => {
updateLabels();
onChange && onChange(formatSelectedData(selected));
}, [selected]);

function buildTreeDataMap() {
return available.reduce((acc, {id: browserId, versions}) => {
const hasChildren = !isEmpty(versions) && versions.length > 1;

acc[buildComplexId(browserId)] = {browserId, isLeaf: !hasChildren};

if (hasChildren) {
versions.forEach(version => {
acc[buildComplexId(browserId, version)] = {browserId, version, isLeaf: true};
});
import { Button, Select, useSelectOptions } from '@gravity-ui/uikit';

const BrowserList = ({available, onChange, selected: selectedProp}) => {
Copy link
Member

Choose a reason for hiding this comment

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

I see that selectedProp was here before, but I think it's bad naming too — it doesn't reflect what the variable holds at all. Let's name it something like selectedBrowsersProp, or you can try to come up with a better name.

and maybe available -> availableBrowsersProp too. This would indicate that these props store values of similar shape.

const getOptions = () => {
const groups = {};
const DEFAULT_GROUP = "other";
let hasNestedOptions = false;
available.forEach(({id: browserId, versions}) => {
if (!versions || versions.length < 2) {
groups[DEFAULT_GROUP] = groups[DEFAULT_GROUP] || [];
groups[DEFAULT_GROUP].push({value: buildComplexId(browserId),
content: <div className='browser-name'>{mkBrowserIcon(browserId)}{buildComplexId(browserId)}</div>});
return;
}
hasNestedOptions = true;
versions.forEach((version) => {
groups[browserId] = groups[browserId] || [];
groups[browserId].push({value: buildComplexId(browserId, version),
content: <div className='browser-name'>{mkBrowserIcon(browserId)}{buildComplexId(browserId, version)}</div>});
})
});
if (!hasNestedOptions) {
return groups[DEFAULT_GROUP];
}
else {
Copy link
Member

Choose a reason for hiding this comment

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

A few remarks about code style:

  • There are almost no blank lines at all. It makes code harder to read. Look at how the code was structured before — we usually divide code into sections (grouped by meaning) with blank lines between them. For instance, it's almost always a good idea to insert a blank line before return statement (unless it's the only statement in the code block e.g. if (...) { return ...; }
  • We usually write else clauses like this } else {. But here we don't need else clause altogether — it just introduces unnecessary level of nesting.

const optionsList = [];
Object.keys(groups).forEach((name) => {
optionsList.push({
label: name,
options: groups[name]
})
})
Copy link
Member

Choose a reason for hiding this comment

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

Here, there's no semicolon. And this is not the only place. Don't we have a linter rule turned on for that? Can you please check? eslint should issue warnings/errors for missing semicolons.

return optionsList;
}

return acc;
}, {});
}

function buidSelectedItems() {
const getMapping = () => {
const mapping = {};
available.forEach(({id: browserId, versions}) => {
if (!versions || !versions.length) {
mapping[buildComplexId(browserId)] = {id: browserId};
return;
}
if (versions.length < 2) {
mapping[buildComplexId(browserId)] = {id: browserId, version: versions[0]};
return;
}
versions.forEach((version) => {
mapping[buildComplexId(browserId, version)] = {id: browserId, version};
})
});
return mapping;
}
const getSelected = () => {
Copy link
Member

Choose a reason for hiding this comment

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

This is not a descriptive&clear name either. By reading getSelected I don't understand at all what this function will return — will it return a single selected string? Or a selected element? Or will it return an array of them?

I suggest something like getSelectedBrowserIds. Feel free to choose the other name, but it should be clear what function does just by looking at its name.

const selectedOptions = [];
if (!selectedProp || !selectedProp.length) {
return [];
}

const selectedItems = selectedProp.map(({id, versions}) => {
if (!versions) {
return [];
selectedProp.forEach(({id: browserId, versions}) => {
if (!versions || versions.length < 2) {
selectedOptions.push(buildComplexId(browserId));
return;
}

const availableNode = available.find((item) => item.id === id);
const shouldDrawOnlyRootNode = get(availableNode, 'versions.length', 1) === 1;

if (shouldDrawOnlyRootNode) {
return buildComplexId(id);
}

return versions.length
? versions.map((version) => buildComplexId(id, version))
: availableNode.versions.map(version => buildComplexId(id, version));
versions.forEach((version) => {
selectedOptions.push(buildComplexId(browserId, version));
})
});

return flatten(selectedItems);
return selectedOptions;
}

function prepareTreeData() {
const mkNode = ({browserId, version, icon}) => ({
icon,
data: {browserId, version},
value: buildComplexId(browserId, version),
label: mkLabel({browserId, version, elements})
});

return available.map(({id: browserId, versions}) => {
const node = mkNode({browserId, icon: mkBrowserIcon(browserId)});

if (!isEmpty(versions) && versions.length > 1) {
node.children = versions.map(version => mkNode({browserId, version}));
const rawOptions = useMemo(getOptions, [available]);
const getOptionsFrom = (optionsData) => {
const allOptionsList = [];
optionsData.forEach((option) => {
if (option.label) {
getOptionsFrom(option.options).forEach((o) => allOptionsList.push(o));
}

return node;
});
else {
allOptionsList.push(option.value);
}
})
return allOptionsList;
}
const allOptions = useMemo(() => getOptionsFrom(rawOptions), [rawOptions]);
const options = useSelectOptions({
options: rawOptions
});
const mapping = useMemo(getMapping, [available]);
const [selected, setSelected] = useState(getSelected());

const selectAll = () => {
setSelected(allOptions);
}

function _selectAll() {
const leaves = Object.keys(treeDataMap).filter(key => treeDataMap[key].isLeaf);
const cb = selected => selected.length !== leaves.length
? leaves
: selected;

setSelected(cb);
const formatSelectedData = () => {
const selectedData = {}
selected.forEach((option) => {
if (!mapping[option] || !mapping[option].id) return;
const {id: browserId, version} = mapping[option];
selectedData[browserId] = selectedData[browserId] || [];
selectedData[browserId].push(version);
})
return Object.entries(selectedData).map(([id, versions]) => ({id, versions: compact(versions)}))
}

function mkLabel({browserId, version, elements}) {
const renderFilter = () => {
const allSelected = selected.length == options.length;
return (
<Label
treeDataMap={treeDataMap}
browserId={browserId}
version={version}
elements={elements}
setSelected={setSelected}
/>
);
<div className='browserlist__filter'>
<Button onClick={selectAll} width='max'>
Select All
</Button>
</div>
)
}

function buildElements() {
const availableVersionsCount = available.reduce((acc, browser) => {
acc[browser.id] = browser.versions ? browser.versions.length : undefined;
return acc;
}, {});

const selectedBrowsers = selected.reduce((acc, value) => {
if (!treeDataMap[value]) {
return acc;
}

const {browserId, version} = treeDataMap[value];

acc[browserId] = acc[browserId] || [];

if (version) {
acc[browserId].push(version);
}

return acc;
}, {});

const elementsNew = [];

for (const browserId in selectedBrowsers) {
const versions = selectedBrowsers[browserId];
if (!versions.length || (availableVersionsCount[browserId] === versions.length)) {
elementsNew.push(browserId);
} else {
elementsNew.push(...versions.map(version => buildComplexId(browserId, version)));
}
const renderOption = (option) => {
const isTheOnlySelected = selected.includes(option.value) && selected.length == 1;
const selectOnly = (e) => {
e.preventDefault();
e.stopPropagation();
setSelected([option.value]);
}

return elementsNew;
}

function updateLabels() {
const newElements = buildElements();

if (get(elements, 'length', 0) !== 1 && get(newElements, 'length', 0) !== 1) {
setElements(newElements);
return;
const selectExcept = (e) => {
e.preventDefault();
e.stopPropagation();
setSelected(allOptions.filter(o => o != option.value));
}

const updatingValues = [
get(newElements, 'length', undefined) === 1 && newElements[0],
get(elements, 'length', undefined) === 1 && elements[0]
];

const nodesFlat = nodes.reduce((arr, node) => {
const children = node.children || [];

arr.push(node);
children.forEach(child => arr.push(child));

return arr;
}, []);

const updatingNodes = nodesFlat.filter(node => {
const {browserId, version} = node.data;

return updatingValues.includes(buildComplexId(browserId, version));
});

updatingNodes.forEach(node => {
const {browserId, version} = node.data;
node.label = mkLabel({browserId, version, elements: newElements});
});

setElements(newElements);
}

function formatSelectedData(selectedItems) {
return chain(selectedItems)
.reduce((acc, value) => {
const {browserId, version} = treeDataMap[value] || {};

if (browserId) {
acc[browserId] = acc[browserId] || [];
acc[browserId].push(version);
}

return acc;
}, {})
.map((versions, id) => ({id, versions: compact(versions)}))
.value();
}

if (isEmpty(available)) {
return (<div></div>);
return (
<div className='browserlist__row'>
Copy link
Member

Choose a reason for hiding this comment

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

Right now when you click on only/except the width of the popup changes, but it shouldn't.

<div className='browserlist__row_content'>
{option.content}
</div>
<Button size='s' onClick={isTheOnlySelected ? selectExcept : selectOnly} className='action-button'>{isTheOnlySelected ? 'Except' : 'Only'}</Button>
</div>
)
}

useEffect(() => {
onChange && onChange(formatSelectedData(selected));
}, [selected]);

return (
<div className="browserlist">
<Popup
target={<ArrayContainer elements={elements} placeholder="Browsers" />}
action="click"
>
<div>
<button className='rct-controls' onClick={selectAll}>
Select all
</button>
<CheckboxTree
nodes={nodes}
checked={selected}
expanded={expanded}
onCheck={setSelected}
onExpand={setExpanded}
/>
</div>
</Popup>
</div>
);
};
<Select
disablePortal
value={selected}
options={options}
multiple={true}
hasCounter
filterable
renderFilter={renderFilter}
renderOption={renderOption}
onUpdate={setSelected}
popupClassName='browserlist__popup'
className='browserlist'
/>
)
}

BrowserList.propTypes = {
available: PropTypes.arrayOf(PropTypes.shape({
Expand Down
Loading
Loading