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

IBX-9227: Restore multipleItemsLimit cehcks in UDW #1396

Merged
merged 1 commit into from
Nov 25, 2024
Merged

IBX-9227: Restore multipleItemsLimit cehcks in UDW #1396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/bundle/Resources/public/scss/_tables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
}
}
}

&--not-selectable {
cursor: not-allowed;
}
}

&__cell:first-child {
Expand Down
13 changes: 5 additions & 8 deletions ...dle/ui-dev/src/modules/universal-discovery/components/content-table/content.table.item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Icon from '../../../common/icon/icon';
import { formatShortDateTime } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/timezone.helper';
import { createCssClassNames } from '../../../common/helpers/css.class.names';
import { loadAccordionData } from '../../services/universal.discovery.service';
import { useSelectedLocationsHelpers } from '../../hooks/useSelectedLocationsHelpers';
import {
RestInfoContext,
CurrentViewContext,
Expand All @@ -17,8 +18,6 @@ import {
ContentTypesMapContext,
SelectedLocationsContext,
MultipleConfigContext,
ContainersOnlyContext,
AllowedContentTypesContext,
RootLocationIdContext,
} from '../../universal.discovery.module';

Expand All @@ -33,12 +32,10 @@ const ContentTableItem = ({ location }) => {
const [, dispatchSelectedLocationsAction] = useContext(SelectedLocationsContext);
const [multiple] = useContext(MultipleConfigContext);
const rootLocationId = useContext(RootLocationIdContext);
const allowedContentTypes = useContext(AllowedContentTypesContext);
const contentTypeInfo = contentTypesMap[location.ContentInfo.Content.ContentType._href];
const containersOnly = useContext(ContainersOnlyContext);
const { isContainer } = contentTypeInfo;
const isNotSelectable =
(containersOnly && !isContainer) || (allowedContentTypes && !allowedContentTypes.includes(contentTypeInfo.identifier));
const { checkIsSelectable, checkIsSelectionBlocked } = useSelectedLocationsHelpers();
const isNotSelectable = !checkIsSelectable(location);
const isSelectionBlocked = checkIsSelectionBlocked(location);
const className = createCssClassNames({
'ibexa-table__row c-content-table-item': true,
'c-content-table-item--marked': markedLocationId === location.id,
Expand Down Expand Up @@ -85,7 +82,7 @@ const ContentTableItem = ({ location }) => {
}
};
const renderToggleSelection = () => {
return <ToggleSelection location={location} multiple={multiple} isHidden={isNotSelectable} />;
return <ToggleSelection location={location} multiple={multiple} isDisabled={isSelectionBlocked} isHidden={isNotSelectable} />;
};

return (
Expand Down
18 changes: 7 additions & 11 deletions src/bundle/ui-dev/src/modules/universal-discovery/components/finder/finder.leaf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import ToggleSelection from '../toggle-selection/toggle.selection';
import Icon from '../../../common/icon/icon';

import { createCssClassNames } from '../../../common/helpers/css.class.names';
import { useSelectedLocationsHelpers } from '../../hooks/useSelectedLocationsHelpers';
import {
MarkedLocationIdContext,
LoadedLocationsMapContext,
ContentTypesMapContext,
SelectedLocationsContext,
MultipleConfigContext,
ContainersOnlyContext,
AllowedContentTypesContext,
} from '../../universal.discovery.module';

const { document } = window;
Expand All @@ -23,15 +22,12 @@ const FinderLeaf = ({ location }) => {
const [markedLocationId, setMarkedLocationId] = useContext(MarkedLocationIdContext);
const [loadedLocationsMap, dispatchLoadedLocationsAction] = useContext(LoadedLocationsMapContext);
const contentTypesMap = useContext(ContentTypesMapContext);
const [selectedLocations, dispatchSelectedLocationsAction] = useContext(SelectedLocationsContext);
const [, dispatchSelectedLocationsAction] = useContext(SelectedLocationsContext);
const [multiple] = useContext(MultipleConfigContext);
const containersOnly = useContext(ContainersOnlyContext);
const allowedContentTypes = useContext(AllowedContentTypesContext);
const contentTypeInfo = contentTypesMap[location.ContentInfo.Content.ContentType._href];
const { isContainer } = contentTypeInfo;
const isSelected = selectedLocations.some((selectedLocation) => selectedLocation.location.id === location.id);
const isNotSelectable =
(containersOnly && !isContainer) || (allowedContentTypes && !allowedContentTypes.includes(contentTypeInfo.identifier));
const { checkIsSelectable, checkIsSelected, checkIsSelectionBlocked } = useSelectedLocationsHelpers();
const isSelected = checkIsSelected(location);
const isNotSelectable = !checkIsSelectable(location);
const isSelectionBlocked = checkIsSelectionBlocked(location);
const markLocation = ({ nativeEvent }) => {
const isSelectionButtonClicked = nativeEvent.target.closest('.c-udw-toggle-selection');
const isMarkedLocationClicked = location.id === markedLocationId;
Expand All @@ -53,7 +49,7 @@ const FinderLeaf = ({ location }) => {
}
};
const renderToggleSelection = () => {
return <ToggleSelection location={location} multiple={multiple} isHidden={isNotSelectable} />;
return <ToggleSelection location={location} multiple={multiple} isDisabled={isSelectionBlocked} isHidden={isNotSelectable} />;
};
const className = createCssClassNames({
'c-finder-leaf': true,
Expand Down
14 changes: 7 additions & 7 deletions src/bundle/ui-dev/src/modules/universal-discovery/components/grid-view/grid.view.item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import ToggleSelection from '../toggle-selection/toggle.selection';
import Thumbnail from '../../../common/thumbnail/thumbnail';

import { createCssClassNames } from '../../../common/helpers/css.class.names';
import { useSelectedLocationsHelpers } from '../../hooks/useSelectedLocationsHelpers';
import {
LoadedLocationsMapContext,
MarkedLocationIdContext,
ContentTypesMapContext,
SelectedLocationsContext,
MultipleConfigContext,
ContainersOnlyContext,
AllowedContentTypesContext,
GridActiveLocationIdContext,
} from '../../universal.discovery.module';

Expand All @@ -25,15 +25,15 @@ const GridViewItem = ({ location, version }) => {
const [markedLocationId, setMarkedLocationId] = useContext(MarkedLocationIdContext);
const [, dispatchLoadedLocationsAction] = useContext(LoadedLocationsMapContext);
const contentTypesMap = useContext(ContentTypesMapContext);
const [selectedLocations, dispatchSelectedLocationsAction] = useContext(SelectedLocationsContext);
const [, dispatchSelectedLocationsAction] = useContext(SelectedLocationsContext);
const [multiple] = useContext(MultipleConfigContext);
const containersOnly = useContext(ContainersOnlyContext);
const allowedContentTypes = useContext(AllowedContentTypesContext);
const contentTypeInfo = contentTypesMap[location.ContentInfo.Content.ContentType._href];
const { isContainer } = contentTypeInfo;
const isSelected = selectedLocations.some((selectedLocation) => selectedLocation.location.id === location.id);
const isNotSelectable =
(containersOnly && !isContainer) || (allowedContentTypes && !allowedContentTypes.includes(contentTypeInfo.identifier));
const { checkIsSelectable, checkIsSelected, checkIsSelectionBlocked } = useSelectedLocationsHelpers();
const isSelected = checkIsSelected(location);
const isNotSelectable = !checkIsSelectable(location);
const isSelectionBlocked = checkIsSelectionBlocked(location);
const className = createCssClassNames({
'ibexa-grid-view-item': true,
'ibexa-grid-view-item--marked': markedLocationId === location.id,
Expand Down Expand Up @@ -68,7 +68,7 @@ const GridViewItem = ({ location, version }) => {
const renderToggleSelection = () => {
return (
<div className="ibexa-grid-view-item__checkbox">
<ToggleSelection location={location} multiple={multiple} isHidden={isNotSelectable} />
<ToggleSelection location={location} multiple={multiple} isDisabled={isSelectionBlocked} isHidden={isNotSelectable} />
</div>
);
};
Expand Down
8 changes: 6 additions & 2 deletions ...le/ui-dev/src/modules/universal-discovery/components/toggle-selection/toggle.selection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
import { createCssClassNames } from '../../../common/helpers/css.class.names';
import { SelectedLocationsContext } from '../../universal.discovery.module';

const ToggleSelection = ({ multiple, location, isHidden }) => {
const ToggleSelection = ({ multiple, location, isDisabled, isHidden }) => {
const [selectedLocations, dispatchSelectedLocationsAction] = useContext(SelectedLocationsContext);
const isSelected = selectedLocations.some((selectedItem) => selectedItem.location.id === location.id);
const className = createCssClassNames({
Expand All @@ -22,17 +22,21 @@ const ToggleSelection = ({ multiple, location, isHidden }) => {
return null;
}

return <input type="checkbox" className={className} checked={isSelected} disabled={isHidden} onChange={toggleSelection} />;
return (
<input type="checkbox" className={className} checked={isSelected} disabled={isDisabled || isHidden} onChange={toggleSelection} />
);
};

ToggleSelection.propTypes = {
location: PropTypes.object.isRequired,
multiple: PropTypes.bool.isRequired,
isHidden: PropTypes.bool,
isDisabled: PropTypes.bool,
};

ToggleSelection.defaultProps = {
isHidden: false,
isDisabled: false,
};

export default ToggleSelection;
6 changes: 4 additions & 2 deletions ...s/universal-discovery/components/tree-item-toggle-selection/tree.item.toggle.selection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ const TreeItemToggleSelection = ({ locationId, isContainer, contentTypeIdentifie
}

const [selectedLocations, dispatchSelectedLocationsAction] = useContext(SelectedLocationsContext);
const [multiple] = useContext(MultipleConfigContext);
const [multiple, multipleItemsLimit] = useContext(MultipleConfigContext);
const containersOnly = useContext(ContainersOnlyContext);
const allowedContentTypes = useContext(AllowedContentTypesContext);
const restInfo = useContext(RestInfoContext);
const isSelected = selectedLocations.some((selectedLocation) => selectedLocation.location.id === locationId);
const isNotSelectable =
(containersOnly && !isContainer) || (allowedContentTypes && !allowedContentTypes.includes(contentTypeIdentifier));
const isSelectionBlocked = multipleItemsLimit !== 0 && !isSelected && selectedLocations.length >= multipleItemsLimit;
const location = {
id: locationId,
};
Expand All @@ -49,7 +51,7 @@ const TreeItemToggleSelection = ({ locationId, isContainer, contentTypeIdentifie

return (
<SelectedLocationsContext.Provider value={[selectedLocations, dispatchSelectedLocationsActionWrapper]}>
<ToggleSelection location={location} multiple={multiple} isHidden={isNotSelectable} />
<ToggleSelection location={location} multiple={multiple} isDisabled={isSelectionBlocked} isHidden={isNotSelectable} />
{isNotSelectable && <div className="c-list-item__prefix-actions-item-empty" />}
</SelectedLocationsContext.Provider>
);
Expand Down
13 changes: 13 additions & 0 deletions src/bundle/ui-dev/src/modules/universal-discovery/helpers/selected.locations.helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export const checkIsSelectable = ({ location, contentTypesMap, allowedContentTypes, containersOnly }) => {
const contentType = contentTypesMap[location.ContentInfo.Content.ContentType._href];
const { isContainer, identifier } = contentType;
const isAllowedContentType = allowedContentTypes?.includes(identifier) ?? true;

return (!containersOnly || isContainer) && isAllowedContentType;
};

export const checkIsSelected = ({ location, selectedLocations }) =>
selectedLocations.some((selectedLocation) => selectedLocation.location.id === location.id);

export const checkIsSelectionBlocked = ({ location, selectedLocations, multipleItemsLimit }) =>
multipleItemsLimit !== 0 && selectedLocations.length >= multipleItemsLimit && !checkIsSelected({ location, selectedLocations });
33 changes: 33 additions & 0 deletions src/bundle/ui-dev/src/modules/universal-discovery/hooks/useSelectedLocationsHelpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { useCallback, useContext } from 'react';

import { checkIsSelectable, checkIsSelected, checkIsSelectionBlocked } from '../helpers/selected.locations.helper';
import {
AllowedContentTypesContext,
ContainersOnlyContext,
ContentTypesMapContext,
MultipleConfigContext,
SelectedLocationsContext,
} from '../universal.discovery.module';

export const useSelectedLocationsHelpers = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I created a hook which wraps helpers so that instead of having to do something like this:

import { checkIsSelectable, checkIsSelected, checkIsSelectionBlocked } from '../../helpers/locations.helper';
import {
    ContentTypesMapContext,
    SelectedLocationsContext,
    MultipleConfigContext,
    ContainersOnlyContext,
    AllowedContentTypesContext,
} from '../../universal.discovery.module';

const contentTypesMap = useContext(ContentTypesMapContext);
const [selectedLocations] = useContext(SelectedLocationsContext);
const [multipleItemsLimit] = useContext(MultipleConfigContext);
const containersOnly = useContext(ContainersOnlyContext);
const allowedContentTypes = useContext(AllowedContentTypesContext);
const isSelected = checkIsSelected({ location, selectedLocations });
const isNotSelectable = !checkIsSelectable({ location, contentTypesMap, allowedContentTypes, containersOnly });
const isSelectionBlocked = checkIsSelectionBlocked({ location, selectedLocations, multipleItemsLimit });

we can do:

import { useSelectedLocationsHelpers } from '../../hooks/useSelectedLocationsHelpers';

const { checkIsSelectable, checkIsSelected, checkIsSelectionBlocked } = useSelectedLocationsHelpers();
const isSelected = checkIsSelected(location);
const isNotSelectable = !checkIsSelectable(location);
const isSelectionBlocked = checkIsSelectionBlocked(location);

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to remove the word Helper from useSelectedLocationsHelpers because at the hooks directory level we have a helpers directory with helpers and this can be confusing

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Łukasz, I think Helper isn't even needed

Copy link
Contributor Author

@tischsoic tischsoic Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think that naming this hook useSelectedLocations would be more confusing because with such a name I would expect that the hook would return data about selected locations. But instead, it returns helpers so I think that useSelectedLocationsHelpers precisely says what we have here and removing Helpers from its name would make it more confusing.

const [, multipleItemsLimit] = useContext(MultipleConfigContext);
const contentTypesMap = useContext(ContentTypesMapContext);
const [selectedLocations] = useContext(SelectedLocationsContext);
const containersOnly = useContext(ContainersOnlyContext);
const allowedContentTypes = useContext(AllowedContentTypesContext);
const checkIsSelectableWrapped = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

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

I have small issue with Wrapped, with current name it can be read as check if IsSelectable wrapped
maybe cachedCheckIsSelectable, or more complex

import { checkIsSelectable: checkIsSelectableHelper } from '../helpers/selected.locations.helper';

...

const checkIsSelectableWrapped = useCallback(
    (location) => checkIsSelectableHelper({ location, contentTypesMap, allowedContentTypes, containersOnly }),
    [contentTypesMap, allowedContentTypes, containersOnly],
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would disagree about cached because there is nothing about caching here, but about wrapping the original helper function so that some parameters are provided by the hook. We can remove useCallback here and the hook would still be valid.

(location) => checkIsSelectable({ location, contentTypesMap, allowedContentTypes, containersOnly }),
[contentTypesMap, allowedContentTypes, containersOnly],
);
const checkIsSelectedWrapped = useCallback((location) => checkIsSelected({ location, selectedLocations }), [selectedLocations]);
const checkIsSelectionBlockedWrapped = useCallback(
(location) => checkIsSelectionBlocked({ location, selectedLocations, multipleItemsLimit }),
[selectedLocations, multipleItemsLimit],
);

return {
checkIsSelectable: checkIsSelectableWrapped,
checkIsSelected: checkIsSelectedWrapped,
checkIsSelectionBlocked: checkIsSelectionBlockedWrapped,
};
};
18 changes: 11 additions & 7 deletions src/bundle/ui-dev/src/modules/universal-discovery/universal.discovery.module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useCallback, useState, createContext, useRef } from 'react';
import React, { useEffect, useCallback, useState, createContext, useRef, useMemo } from 'react';
import PropTypes from 'prop-types';

import Icon from '../common/icon/icon';
Expand Down Expand Up @@ -268,13 +268,17 @@ const UniversalDiscoveryModule = (props) => {
loadContentInfo({ ...restInfo, contentId, signal }, (response) => resolve(response));
});
};
const contentTypesMapGlobal = Object.values(adminUiConfig.contentTypes).reduce((contentTypesMap, contentTypesGroup) => {
contentTypesGroup.forEach((contentType) => {
contentTypesMap[contentType.href] = contentType;
});
const contentTypesMapGlobal = useMemo(
() =>
Object.values(adminUiConfig.contentTypes).reduce((contentTypesMap, contentTypesGroup) => {
contentTypesGroup.forEach((contentType) => {
contentTypesMap[contentType.href] = contentType;
});

return contentTypesMap;
}, {});
return contentTypesMap;
}, {}),
[adminUiConfig.contentTypes],
);
const onConfirm = useCallback(
(selectedItems = selectedLocations) => {
loadVersions().then((locationsWithVersions) => {
Expand Down
Loading