feat: display only one card action overflow menu (#2427)

Instead of stopping whole click event propagation from actions to card element, specifically stop click event if the source target is actions menu.
This commit is contained in:
Navin Karkera
2025-09-09 03:17:17 +05:30
committed by GitHub
parent 87239ab723
commit 720b591add
7 changed files with 121 additions and 93 deletions

View File

@@ -62,7 +62,7 @@ import {
* @param {string} courseId - ID of the course
* @returns {Object} - Object containing fetch course outline index query success or failure status
*/
export function fetchCourseOutlineIndexQuery(courseId: string): object {
export function fetchCourseOutlineIndexQuery(courseId: string): (dispatch: any) => Promise<void> {
return async (dispatch) => {
dispatch(updateOutlineIndexLoadingStatus({ status: RequestStatus.IN_PROGRESS }));

View File

@@ -7,11 +7,12 @@ import {
Stack,
} from '@openedx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';
import { getItemIcon, getComponentStyleColor } from '@src/generic/block-type-utils';
import ComponentCount from '@src/generic/component-count';
import TagCount from '@src/generic/tag-count';
import { BlockTypeLabel, type ContentHitTags, Highlight } from '@src/search-manager';
import { skipIfUnwantedTarget } from '@src/utils';
import messages from './messages';
import { getItemIcon, getComponentStyleColor } from '../../generic/block-type-utils';
import ComponentCount from '../../generic/component-count';
import TagCount from '../../generic/tag-count';
import { BlockTypeLabel, type ContentHitTags, Highlight } from '../../search-manager';
type BaseCardProps = {
itemType: string;
@@ -52,7 +53,7 @@ const BaseCard = ({
<Container className="library-item-card selected">
<Card
isClickable
onClick={onSelect}
onClick={(e: React.MouseEvent) => skipIfUnwantedTarget(e, onSelect)}
onKeyDown={(e: React.KeyboardEvent) => {
if (['Enter', ' '].includes(e.key)) {
onSelect();
@@ -65,12 +66,13 @@ const BaseCard = ({
title={
<Icon src={itemIcon} className="library-item-header-icon" />
}
actions={
// Wrap the actions in a div to prevent the card from being clicked when the actions are clicked
/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,
jsx-a11y/no-static-element-interactions */
<div onClick={(e) => e.stopPropagation()}>{actions}</div>
}
actions={(
<div
// Prevent card being clicked when actions menu are clicked
className="stop-event-propagation"
>{actions}
</div>
)}
/>
<Card.Body className="w-100">
<Card.Section>

View File

@@ -24,7 +24,7 @@ import { ToastContext } from '../../generic/toast-context';
import TagCount from '../../generic/tag-count';
import { useLibraryRoutes } from '../routes';
import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext';
import { useRunOnNextRender } from '../../utils';
import { skipIfUnwantedTarget, useRunOnNextRender } from '../../utils';
import { ContainerMenu } from '../containers/ContainerCard';
interface LibraryContainerChildrenProps {
@@ -76,7 +76,7 @@ const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps)
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
<div
// Prevent parent card from being clicked.
onClick={(e) => e.stopPropagation()}
className="stop-event-propagation"
>
<InplaceTextEditor
onSave={handleSaveDisplayName}
@@ -90,8 +90,7 @@ const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps)
direction="horizontal"
gap={3}
// Prevent parent card from being clicked.
/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */
onClick={(e) => e.stopPropagation()}
className="stop-event-propagation"
>
{!showOnlyPublished && container.hasUnpublishedChanges && (
<Badge
@@ -230,7 +229,7 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont
borderLeft: '8px solid #E1DDDB',
}}
isClickable={!readOnly}
onClick={(e) => handleChildClick(child, e.detail)}
onClick={(e) => skipIfUnwantedTarget(e, (event) => handleChildClick(child, event.detail))}
onKeyDown={(e) => {
if (e.key === 'Enter') {
handleChildClick(child, 1);

View File

@@ -7,16 +7,18 @@ import classNames from 'classnames';
import {
useCallback, useContext, useEffect, useState,
} from 'react';
import { blockTypes } from '../../editors/data/constants/app';
import DraggableList, { SortableItem } from '../../generic/DraggableList';
import { blockTypes } from '@src/editors/data/constants/app';
import DraggableList, { SortableItem } from '@src/generic/DraggableList';
import ErrorAlert from '../../generic/alert-error';
import { getItemIcon } from '../../generic/block-type-utils';
import { COMPONENT_TYPES } from '../../generic/block-type-utils/constants';
import { IframeProvider } from '../../generic/hooks/context/iFrameContext';
import { InplaceTextEditor } from '../../generic/inplace-text-editor';
import Loading from '../../generic/Loading';
import TagCount from '../../generic/tag-count';
import ErrorAlert from '@src/generic/alert-error';
import { getItemIcon } from '@src/generic/block-type-utils';
import { COMPONENT_TYPES } from '@src/generic/block-type-utils/constants';
import { IframeProvider } from '@src/generic/hooks/context/iFrameContext';
import { InplaceTextEditor } from '@src/generic/inplace-text-editor';
import Loading from '@src/generic/Loading';
import TagCount from '@src/generic/tag-count';
import { ToastContext } from '@src/generic/toast-context';
import { skipIfUnwantedTarget, useRunOnNextRender } from '@src/utils';
import { useLibraryContext } from '../common/context/LibraryContext';
import ComponentMenu from '../components';
import { LibraryBlockMetadata } from '../data/api';
@@ -28,9 +30,7 @@ import {
import { LibraryBlock } from '../LibraryBlock';
import messages from './messages';
import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext';
import { ToastContext } from '../../generic/toast-context';
import { canEditComponent } from '../components/ComponentEditorModal';
import { useRunOnNextRender } from '../../utils';
/** Components that need large min height in preview */
const LARGE_COMPONENTS = [
@@ -91,9 +91,7 @@ const BlockHeader = ({ block, readOnly }: ComponentBlockProps) => {
<Stack
direction="horizontal"
gap={2}
className="font-weight-bold"
// Prevent parent card from being clicked.
onClick={(e) => e.stopPropagation()}
className="font-weight-bold stop-event-propagation"
>
<Icon src={getItemIcon(block.blockType)} />
<InplaceTextEditor
@@ -106,8 +104,7 @@ const BlockHeader = ({ block, readOnly }: ComponentBlockProps) => {
<Stack
direction="horizontal"
gap={3}
// Prevent parent card from being clicked.
onClick={(e) => e.stopPropagation()}
className="stop-event-propagation"
>
{!showOnlyPublished && block.hasUnpublishedChanges && (
<Badge
@@ -185,7 +182,7 @@ const ComponentBlock = ({ block, readOnly, isDragging }: ComponentBlockProps) =>
borderBottom: 'solid 1px #E1DDDB',
}}
isClickable={!readOnly}
onClick={(e) => handleComponentSelection(e.detail)}
onClick={(e) => skipIfUnwantedTarget(e, (event) => handleComponentSelection(event.detail))}
onKeyDown={(e) => {
if (e.key === 'Enter') {
handleComponentSelection(e.detail);

View File

@@ -1,25 +0,0 @@
import React, { useMemo } from 'react';
import PropTypes from 'prop-types';
export const PagesAndResourcesContext = React.createContext({});
const PagesAndResourcesProvider = ({ courseId, children }) => {
const contextValue = useMemo(() => ({
courseId,
path: `/course/${courseId}/pages-and-resources`,
}), []);
return (
<PagesAndResourcesContext.Provider
value={contextValue}
>
{children}
</PagesAndResourcesContext.Provider>
);
};
PagesAndResourcesProvider.propTypes = {
courseId: PropTypes.string.isRequired,
children: PropTypes.node.isRequired,
};
export default PagesAndResourcesProvider;

View File

@@ -0,0 +1,28 @@
import React, { useMemo } from 'react';
interface PagesAndResourcesContextData {
courseId?: string;
path?: string;
}
export const PagesAndResourcesContext = React.createContext<PagesAndResourcesContextData>({});
interface PagesAndResourcesProviderProps {
courseId: string;
children: React.ReactNode,
}
const PagesAndResourcesProvider = ({ courseId, children }: PagesAndResourcesProviderProps) => {
const contextValue = useMemo(() => ({
courseId,
path: `/course/${courseId}/pages-and-resources`,
}), []);
return (
<PagesAndResourcesContext.Provider
value={contextValue}
>
{children}
</PagesAndResourcesContext.Provider>
);
};
export default PagesAndResourcesProvider;

View File

@@ -1,4 +1,4 @@
import { useState, useContext, useEffect } from 'react';
import React, { useState, useContext, useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useMediaQuery } from 'react-responsive';
import * as Yup from 'yup';
@@ -6,6 +6,8 @@ import { snakeCase } from 'lodash/string';
import moment from 'moment';
import { getConfig, getPath } from '@edx/frontend-platform';
import type { Dispatch, AnyAction } from 'redux';
import type { TypeOfShape } from 'yup/lib/object';
import { RequestStatus } from './data/constants';
import { getCourseAppSettingValue, getLoadingStatus } from './pages-and-resources/data/selectors';
import { fetchCourseAppSettings, updateCourseAppSetting } from './pages-and-resources/data/thunks';
@@ -15,7 +17,11 @@ import {
} from './pages-and-resources/discussions/app-config-form/utils';
import { DATE_TIME_FORMAT } from './constants';
export const executeThunk = async (thunk, dispatch, getState) => {
export const executeThunk = async (
thunk: (dispatch: any, state?: any) => Promise<any>,
dispatch: Dispatch<AnyAction>,
getState?: any,
) => {
await thunk(dispatch, getState);
await new Promise(setImmediate);
};
@@ -28,7 +34,7 @@ export function useIsDesktop() {
return useMediaQuery({ query: '(min-width: 992px)' });
}
export function convertObjectToSnakeCase(obj, unpacked = false) {
export function convertObjectToSnakeCase(obj: Object, unpacked = false) {
return Object.keys(obj).reduce((snakeCaseObj, key) => {
const snakeCaseKey = snakeCase(key);
const value = unpacked ? obj[key] : { value: obj[key] };
@@ -39,7 +45,7 @@ export function convertObjectToSnakeCase(obj, unpacked = false) {
}, {});
}
export function deepConvertingKeysToCamelCase(obj) {
export function deepConvertingKeysToCamelCase(obj: any[] | Object | null) {
if (typeof obj !== 'object' || obj === null) {
return obj;
}
@@ -50,13 +56,13 @@ export function deepConvertingKeysToCamelCase(obj) {
const camelCaseObj = {};
Object.keys(obj).forEach((key) => {
const camelCaseKey = key.replace(/_([a-z])/g, (match, p1) => p1.toUpperCase());
const camelCaseKey = key.replace(/_([a-z])/g, (_, p1) => p1.toUpperCase());
camelCaseObj[camelCaseKey] = deepConvertingKeysToCamelCase(obj[key]);
});
return camelCaseObj;
}
export function deepConvertingKeysToSnakeCase(obj) {
export function deepConvertingKeysToSnakeCase(obj: any[] | Object | null) {
if (typeof obj !== 'object' || obj === null) {
return obj;
}
@@ -73,11 +79,11 @@ export function deepConvertingKeysToSnakeCase(obj) {
return snakeCaseObj;
}
export function transformKeysToCamelCase(obj) {
return obj.key.replace(/_([a-z])/g, (match, letter) => letter.toUpperCase());
export function transformKeysToCamelCase(obj: { key: any; }) {
return obj.key.replace(/_([a-z])/g, (_: any, letter: string) => letter.toUpperCase());
}
export function parseArrayOrObjectValues(obj) {
export function parseArrayOrObjectValues(obj: { [s: string]: string; } | ArrayLike<string>) {
const result = {};
Object.entries(obj).forEach(([key, value]) => {
@@ -97,10 +103,8 @@ export function parseArrayOrObjectValues(obj) {
/**
* Create a correct inner path depend on config PUBLIC_PATH.
* @param {string} checkPath - the internal route path that is validated
* @returns {string} - the correct internal route path
*/
export const createCorrectInternalRoute = (checkPath) => {
export const createCorrectInternalRoute = (checkPath: string): string => {
let basePath = getPath(getConfig().PUBLIC_PATH);
if (basePath.endsWith('/')) {
@@ -114,7 +118,7 @@ export const createCorrectInternalRoute = (checkPath) => {
return checkPath;
};
export function getPagePath(courseId, isMfePageEnabled, urlParameter) {
export function getPagePath(courseId: string | undefined, isMfePageEnabled: string, urlParameter: string) {
if (isMfePageEnabled === 'true') {
if (urlParameter === 'tabs') {
return `/course/${courseId}/pages-and-resources`;
@@ -124,7 +128,7 @@ export function getPagePath(courseId, isMfePageEnabled, urlParameter) {
return `${getConfig().STUDIO_BASE_URL}/${urlParameter}/${courseId}`;
}
export function useAppSetting(settingName) {
export function useAppSetting(settingName: string) {
const dispatch = useDispatch();
const { courseId } = useContext(PagesAndResourcesContext);
const settingValue = useSelector(getCourseAppSettingValue(settingName));
@@ -138,15 +142,19 @@ export function useAppSetting(settingName) {
}
}, [courseId]);
const saveSetting = async (value) => dispatch(updateCourseAppSetting(courseId, settingName, value));
const saveSetting = async (value: any) => dispatch(updateCourseAppSetting(courseId, settingName, value));
return [settingValue, saveSetting];
}
export const getLabelById = (options, id) => {
export const getLabelById = (options: any[], id: any) => {
const foundOption = options.find((option) => option.id === id);
return foundOption ? foundOption.label : '';
};
interface YupTestContextExtended {
originalValue?: unknown;
}
/**
* Adds additional validation methods to Yup.
*/
@@ -156,9 +164,9 @@ export function setupYupExtensions() {
// Credit: https://github.com/jquense/yup/issues/345#issuecomment-717400071
Yup.addMethod(Yup.array, 'uniqueProperty', function uniqueProperty(property, message) {
return this.test('unique', '', function testUniqueness(list) {
const errors = [];
const errors: Yup.ValidationError[] = [];
list.forEach((item, index) => {
list?.forEach((item, index) => {
const propertyValue = item[property];
if (propertyValue && list.filter(entry => entry[property] === propertyValue).length > 1) {
@@ -184,13 +192,15 @@ export function setupYupExtensions() {
if (!discussionTopic || !discussionTopic[propertyName]) {
return true;
}
const isDuplicate = this.parent.filter(topic => topic !== discussionTopic)
.some(topic => topic[propertyName]?.toLowerCase() === discussionTopic[propertyName].toLowerCase());
const isDuplicate = this.parent.filter((topic: TypeOfShape<any>) => topic !== discussionTopic)
.some((
topic: { [x: string]: string; },
) => topic[propertyName]?.toLowerCase() === discussionTopic[propertyName].toLowerCase());
if (isDuplicate) {
throw this.createError({
path: `${this.path}.${propertyName}`,
error: message,
message,
});
}
return true;
@@ -212,7 +222,7 @@ export function setupYupExtensions() {
const startDateTime = decodeDateTime(this.parent.startDate, startOfDayTime(this.parent.startTime));
const endDateTime = decodeDateTime(this.parent.endDate, endOfDayTime(this.parent.endTime));
let isInvalidStartDateTime;
let isInvalidStartDateTime: boolean = false;
if (type === 'date') {
isInvalidStartDateTime = startDateTime.isAfter(endDateTime);
@@ -223,7 +233,7 @@ export function setupYupExtensions() {
if (isInvalidStartDateTime) {
throw this.createError({
path: `${this.path}`,
error: message,
message,
});
}
return true;
@@ -232,21 +242,22 @@ export function setupYupExtensions() {
Yup.addMethod(Yup.string, 'checkFormat', function checkFormat(message, type) {
return this.test('isValidFormat', message, function isValidFormat() {
if (!this.originalValue) {
const { originalValue } = this as Yup.TestContext & YupTestContextExtended;
if (!originalValue) {
return true;
}
let isValid;
let isValid: boolean = false;
if (type === 'date') {
isValid = hasValidDateFormat(this.originalValue);
isValid = hasValidDateFormat(originalValue);
} else if (type === 'time') {
isValid = hasValidTimeFormat(this.originalValue);
isValid = hasValidTimeFormat(originalValue);
}
if (!isValid) {
throw this.createError({
path: `${this.path}`,
error: message,
message,
});
}
return true;
@@ -254,7 +265,7 @@ export function setupYupExtensions() {
});
}
export const convertToDateFromString = (dateStr) => {
export const convertToDateFromString = (dateStr: string) => {
/**
* Convert UTC to local time for react-datepicker
* Note: react-datepicker has a bug where it only interacts with local time
@@ -265,12 +276,12 @@ export const convertToDateFromString = (dateStr) => {
return '';
}
const stripTimeZone = (stringValue) => stringValue.substring(0, 19);
const stripTimeZone = (stringValue: string) => stringValue.substring(0, 19);
return moment(stripTimeZone(String(dateStr))).toDate();
};
export const convertToStringFromDate = (date) => {
export const convertToStringFromDate = (date: moment.MomentInput) => {
/**
* Convert local time to UTC from react-datepicker
* Note: react-datepicker has a bug where it only interacts with local time
@@ -284,13 +295,13 @@ export const convertToStringFromDate = (date) => {
return moment(date).format(DATE_TIME_FORMAT);
};
export const isValidDate = (date) => {
export const isValidDate = (date: moment.MomentInput) => {
const formattedValue = convertToStringFromDate(date).split('T')[0];
return Boolean(formattedValue.length <= 10);
};
export const getFileSizeToClosestByte = (fileSize) => {
export const getFileSizeToClosestByte = (fileSize: any) => {
let divides = 0;
let size = fileSize;
while (size > 1000 && divides < 4) {
@@ -306,7 +317,7 @@ export const getFileSizeToClosestByte = (fileSize) => {
* A generic hook to run callback on next render cycle.
* @param {} callback - Callback function that needs to be run later
*/
export const useRunOnNextRender = (callback) => {
export const useRunOnNextRender = (callback: () => void) => {
const [scheduled, setScheduled] = useState(false);
useEffect(() => {
@@ -320,3 +331,19 @@ export const useRunOnNextRender = (callback) => {
return () => setScheduled(true);
};
/**
* Checks if the click event originated from an element with the stop-event-propagation class.
* If so, return without further processing.
*/
export const skipIfUnwantedTarget = (
e: React.MouseEvent,
onClick: (e: React.MouseEvent) => void,
selector?: string,
) => {
const target = e.target as HTMLElement;
if (target && target.closest(selector || '.stop-event-propagation')) {
return;
}
onClick(e);
};