-
Notifications
You must be signed in to change notification settings - Fork 0
feat: added soft delete functionality #6
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -78,10 +78,13 @@ const ActionsDropdown = ({ | |||||||||
| size="inline" | ||||||||||
| onClick={() => { | ||||||||||
| close(); | ||||||||||
| handleActions(action.action); | ||||||||||
| if (!action.disabled) { | ||||||||||
| handleActions(action.action); | ||||||||||
| } | ||||||||||
|
Comment on lines
+81
to
+83
|
||||||||||
| if (!action.disabled) { | |
| handleActions(action.action); | |
| } | |
| handleActions(action.action); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,17 @@ | ||
| import React, { | ||
| useCallback, useRef, useState, | ||
| useCallback, useEffect, useRef, useState, | ||
| } from 'react'; | ||
| import ReactDOM from 'react-dom'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| import { | ||
| Button, Dropdown, Icon, IconButton, ModalPopup, useToggle, | ||
| } from '@openedx/paragon'; | ||
| import { MoreHoriz } from '@openedx/paragon/icons'; | ||
| import { ChevronRight, MoreHoriz } from '@openedx/paragon/icons'; | ||
|
|
||
| import { useIntl } from '@edx/frontend-platform/i18n'; | ||
|
|
||
| import { useLearnerActions } from './utils'; | ||
| import { useLearnerActionsMenu } from './utils'; | ||
|
|
||
| const LearnerActionsDropdown = ({ | ||
| actionHandlers, | ||
|
|
@@ -21,14 +22,16 @@ const LearnerActionsDropdown = ({ | |
| const intl = useIntl(); | ||
| const [isOpen, open, close] = useToggle(false); | ||
| const [target, setTarget] = useState(null); | ||
| const actions = useLearnerActions(userHasBulkDeletePrivileges); | ||
| const [activeSubmenu, setActiveSubmenu] = useState(null); | ||
| const menuItems = useLearnerActionsMenu(intl, userHasBulkDeletePrivileges); | ||
|
|
||
| const handleActions = useCallback((action) => { | ||
| const actionFunction = actionHandlers[action]; | ||
| if (actionFunction) { | ||
| actionFunction(); | ||
| close(); | ||
| } | ||
| }, [actionHandlers]); | ||
| }, [actionHandlers, close]); | ||
|
|
||
| const onClickButton = useCallback((event) => { | ||
| event.preventDefault(); | ||
|
|
@@ -39,8 +42,15 @@ const LearnerActionsDropdown = ({ | |
| const onCloseModal = useCallback(() => { | ||
| close(); | ||
| setTarget(null); | ||
| setActiveSubmenu(null); | ||
| }, [close]); | ||
|
|
||
| // Cleanup portal on unmount to prevent memory leaks and orphaned DOM nodes | ||
| useEffect(() => () => { | ||
| setTarget(null); | ||
| setActiveSubmenu(null); | ||
| }, []); | ||
|
|
||
| return ( | ||
| <> | ||
| <IconButton | ||
|
|
@@ -53,41 +63,69 @@ const LearnerActionsDropdown = ({ | |
| iconClassNames={dropDownIconSize ? 'dropdown-icon-dimensions' : ''} | ||
| /> | ||
| <div className="actions-dropdown"> | ||
| <ModalPopup | ||
| onClose={onCloseModal} | ||
| positionRef={target} | ||
| isOpen={isOpen} | ||
| placement="bottom-start" | ||
| > | ||
| <div | ||
| className="bg-white shadow d-flex flex-column mt-1" | ||
| data-testid="learner-actions-dropdown-modal-popup" | ||
| {isOpen && ReactDOM.createPortal( | ||
| <ModalPopup | ||
| onClose={onCloseModal} | ||
| positionRef={target} | ||
| isOpen={isOpen} | ||
| placement="bottom-start" | ||
| style={{ zIndex: 9998 }} | ||
| > | ||
| {actions.map(action => ( | ||
| <React.Fragment key={action.id}> | ||
| <Dropdown.Item | ||
| as={Button} | ||
| variant="tertiary" | ||
| size="inline" | ||
| onClick={() => { | ||
| close(); | ||
| handleActions(action.action); | ||
| }} | ||
| className="d-flex justify-content-start actions-dropdown-item" | ||
| data-testId={action.id} | ||
| <div | ||
| className="bg-white shadow d-flex flex-column mt-1" | ||
| data-testid="learner-actions-dropdown-modal-popup" | ||
| style={{ position: 'relative', zIndex: 9998 }} | ||
| > | ||
| {menuItems.map(item => ( | ||
| <div | ||
| key={item.id} | ||
| className="position-relative" | ||
| onMouseEnter={() => setActiveSubmenu(item.id)} | ||
| onMouseLeave={() => setActiveSubmenu(null)} | ||
| style={{ zIndex: 2 }} | ||
| > | ||
| <Icon | ||
| src={action.icon} | ||
| className="icon-size-24" | ||
| /> | ||
| <span className="font-weight-normal ml-2"> | ||
| {action.label.defaultMessage} | ||
| </span> | ||
| </Dropdown.Item> | ||
| </React.Fragment> | ||
| ))} | ||
| </div> | ||
| </ModalPopup> | ||
| <Dropdown.Item | ||
| as={Button} | ||
| variant="tertiary" | ||
| size="inline" | ||
| className="d-flex justify-content-between align-items-center actions-dropdown-item" | ||
| data-testid={item.id} | ||
| > | ||
| <div className="d-flex align-items-center"> | ||
| <span className="font-weight-normal"> | ||
| {item.label} | ||
| </span> | ||
| </div> | ||
| <Icon | ||
| src={ChevronRight} | ||
| className="icon-size-16" | ||
| /> | ||
| </Dropdown.Item> | ||
| {activeSubmenu === item.id && ( | ||
| <div className="bg-white learner-submenu-container"> | ||
| {item.submenu.map(subItem => ( | ||
| <Dropdown.Item | ||
| key={subItem.id} | ||
| as={Button} | ||
| variant="tertiary" | ||
| size="inline" | ||
| onClick={() => handleActions(subItem.action)} | ||
| className="d-flex justify-content-start actions-dropdown-item" | ||
| data-testid={subItem.id} | ||
| > | ||
| <span className="font-weight-normal"> | ||
| {subItem.label} | ||
| </span> | ||
| </Dropdown.Item> | ||
| ))} | ||
| </div> | ||
| )} | ||
|
Comment on lines
+104
to
+122
|
||
| </div> | ||
| ))} | ||
| </div> | ||
| </ModalPopup>, | ||
| document.body, | ||
| )} | ||
Alam-2U marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+66
to
+128
|
||
| </div> | ||
| </> | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filter on line 137 uses
!f.hasSeparatorto filter out items with separators, but this creates a coupling between the filter structure and the rendering logic. If a filter object doesn't have thehasSeparatorproperty defined, it will be treated as if it has no separator (sinceundefinedis falsy). This implicit behavior could lead to bugs if filter definitions are incomplete. Consider being more explicit, such as checkingf.hasSeparator !== true.