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

Jump to bottom

Fix tags and schedule edit for parental control #6231

Merged
merged 12 commits into from
Oct 20, 2024
+75 −93
Merged

Fix tags and schedule edit for parental control #6231

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
956f9bf
Fix tags and schedule edit for parental control
gnattu Oct 20, 2024
88f9c3d
Don’t use FIXME
gnattu Oct 20, 2024
7eda53f
Use anchor for onClick
gnattu Oct 20, 2024
84f7cf1
Use button
gnattu Oct 20, 2024
d9786d4
Unset button style
gnattu Oct 20, 2024
0ad0a61
Add libraryMenu as dep
gnattu Oct 20, 2024
3ae148f
Use index as key for access schedule
gnattu Oct 20, 2024
95e935a
Fix lint
gnattu Oct 20, 2024
bb17d58
Use daysofweek and hours as key
gnattu Oct 20, 2024
5b687f6
Unbind event listeners when unmounting
gnattu Oct 20, 2024
a1acb57
Fix lint
gnattu Oct 20, 2024
e17d50f
Cleanup functions
gnattu Oct 20, 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
125 changes: 51 additions & 74 deletions src/apps/dashboard/routes/users/parentalcontrol.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@
const [ blockedTags, setBlockedTags ] = useState<string[]>([]);
const libraryMenu = useMemo(async () => ((await import('../../../../scripts/libraryMenu')).default), []);

// The following are meant to be reset on each render.
// These are to prevent multiple callbacks to be added to a single element in one render as useEffect may be executed multiple times in each render.
viown marked this conversation as resolved.
Show resolved Hide resolved
let allowedTagsPopupCallback: (() => void) | null = null;
let blockedTagsPopupCallback: (() => void) | null = null;
let accessSchedulesPopupCallback: (() => void) | null = null;
let formSubmissionCallback: ((e: Event) => void) | null = null;

const element = useRef<HTMLDivElement>(null);

const populateRatings = useCallback((allParentalRatings: ParentalRating[]) => {
Expand Down Expand Up @@ -146,48 +153,6 @@
blockUnratedItems.dispatchEvent(new CustomEvent('create'));
}, []);

const loadAllowedTags = useCallback((tags: string[]) => {
const page = element.current;

if (!page) {
console.error('[userparentalcontrol] Unexpected null page reference');
return;
}

setAllowedTags(tags);

const allowedTagsElem = page.querySelector('.allowedTags') as HTMLDivElement;

for (const btnDeleteTag of allowedTagsElem.querySelectorAll('.btnDeleteTag')) {
btnDeleteTag.addEventListener('click', function () {
const tag = btnDeleteTag.getAttribute('data-tag');
const newTags = tags.filter(t => t !== tag);
loadAllowedTags(newTags);
});
}
}, []);

const loadBlockedTags = useCallback((tags: string[]) => {
const page = element.current;

if (!page) {
console.error('[userparentalcontrol] Unexpected null page reference');
return;
}

setBlockedTags(tags);

const blockedTagsElem = page.querySelector('.blockedTags') as HTMLDivElement;

for (const btnDeleteTag of blockedTagsElem.querySelectorAll('.btnDeleteTag')) {
btnDeleteTag.addEventListener('click', function () {
const tag = btnDeleteTag.getAttribute('data-tag');
const newTags = tags.filter(t => t !== tag);
loadBlockedTags(newTags);
});
}
}, []);

const loadUser = useCallback((user: UserDto, allParentalRatings: ParentalRating[]) => {
const page = element.current;

Expand All @@ -200,8 +165,8 @@
void libraryMenu.then(menu => menu.setTitle(user.Name));
loadUnratedItems(user);

loadAllowedTags(user.Policy?.AllowedTags || []);
loadBlockedTags(user.Policy?.BlockedTags || []);
setAllowedTags(user.Policy?.AllowedTags || []);
setBlockedTags(user.Policy?.BlockedTags || []);
populateRatings(allParentalRatings);

let ratingValue = '';
Expand All @@ -222,7 +187,7 @@
}
setAccessSchedules(user.Policy?.AccessSchedules || []);
loading.hide();
}, [loadAllowedTags, loadBlockedTags, loadUnratedItems, populateRatings]);
}, [setAllowedTags, setBlockedTags, loadUnratedItems, populateRatings]);

Check warning on line 190 in src/apps/dashboard/routes/users/parentalcontrol.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

React Hook useCallback has a missing dependency: 'libraryMenu'. Either include it or remove the dependency array
github-actions[bot] marked this conversation as resolved.
Show resolved Hide resolved

const loadData = useCallback(() => {
if (!userId) {
Expand Down Expand Up @@ -296,7 +261,7 @@

if (tags.indexOf(value) == -1) {
tags.push(value);
loadAllowedTags(tags);
setAllowedTags(tags);
}
}).catch(() => {
// prompt closed
Expand All @@ -317,7 +282,7 @@

if (tags.indexOf(value) == -1) {
tags.push(value);
loadBlockedTags(tags);
setBlockedTags(tags);
}
}).catch(() => {
// prompt closed
Expand Down Expand Up @@ -348,45 +313,39 @@
return false;
};

(page.querySelector('#btnAddSchedule') as HTMLButtonElement).addEventListener('click', function () {
// FIXME: The following is still hacky and should migrate to pure react implementation for callbacks in the future

Check warning on line 316 in src/apps/dashboard/routes/users/parentalcontrol.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

Unexpected 'fixme' comment: 'FIXME: The following is still hacky and...'
github-actions[bot] marked this conversation as resolved.
Show resolved Hide resolved
if (accessSchedulesPopupCallback) {
(page.querySelector('#btnAddSchedule') as HTMLButtonElement).removeEventListener('click', accessSchedulesPopupCallback);
}
accessSchedulesPopupCallback = function () {

Check warning on line 320 in src/apps/dashboard/routes/users/parentalcontrol.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

Assignments to the 'accessSchedulesPopupCallback' variable from inside React Hook useEffect will be lost after each render. To preserve the value over time, store it in a useRef Hook and keep the mutable value in the '.current' property. Otherwise, you can move this variable directly inside useEffect
github-actions[bot] marked this conversation as resolved.
Show resolved Hide resolved
showSchedulePopup({
Id: 0,
UserId: '',
DayOfWeek: DynamicDayOfWeek.Sunday,
StartHour: 0,
EndHour: 0
}, -1);
});

(page.querySelector('#btnAddAllowedTag') as HTMLButtonElement).addEventListener('click', function () {
showAllowedTagPopup();
});

(page.querySelector('#btnAddBlockedTag') as HTMLButtonElement).addEventListener('click', function () {
showBlockedTagPopup();
});

(page.querySelector('.userParentalControlForm') as HTMLFormElement).addEventListener('submit', onSubmit);
}, [loadAllowedTags, loadBlockedTags, loadData, userId]);

useEffect(() => {
const page = element.current;
};
(page.querySelector('#btnAddSchedule') as HTMLButtonElement).addEventListener('click', accessSchedulesPopupCallback);

if (!page) {
console.error('[userparentalcontrol] Unexpected null page reference');
return;
if (allowedTagsPopupCallback) {
(page.querySelector('#btnAddAllowedTag') as HTMLButtonElement).removeEventListener('click', allowedTagsPopupCallback);
}
allowedTagsPopupCallback = showAllowedTagPopup;

Check warning on line 334 in src/apps/dashboard/routes/users/parentalcontrol.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

Assignments to the 'allowedTagsPopupCallback' variable from inside React Hook useEffect will be lost after each render. To preserve the value over time, store it in a useRef Hook and keep the mutable value in the '.current' property. Otherwise, you can move this variable directly inside useEffect
github-actions[bot] marked this conversation as resolved.
Show resolved Hide resolved
(page.querySelector('#btnAddAllowedTag') as HTMLButtonElement).addEventListener('click', allowedTagsPopupCallback);

const accessScheduleList = page.querySelector('.accessScheduleList') as HTMLDivElement;
if (blockedTagsPopupCallback) {
(page.querySelector('#btnAddBlockedTag') as HTMLButtonElement).removeEventListener('click', blockedTagsPopupCallback);
}
blockedTagsPopupCallback = showBlockedTagPopup;

Check warning on line 340 in src/apps/dashboard/routes/users/parentalcontrol.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

Assignments to the 'blockedTagsPopupCallback' variable from inside React Hook useEffect will be lost after each render. To preserve the value over time, store it in a useRef Hook and keep the mutable value in the '.current' property. Otherwise, you can move this variable directly inside useEffect
github-actions[bot] marked this conversation as resolved.
Show resolved Hide resolved
(page.querySelector('#btnAddBlockedTag') as HTMLButtonElement).addEventListener('click', blockedTagsPopupCallback);

for (const btnDelete of accessScheduleList.querySelectorAll('.btnDelete')) {
btnDelete.addEventListener('click', function () {
const index = parseInt(btnDelete.getAttribute('data-index') ?? '0', 10);
const newindex = accessSchedules.filter((_e, i) => i != index);
setAccessSchedules(newindex);
});
if (formSubmissionCallback) {
(page.querySelector('.userParentalControlForm') as HTMLFormElement).removeEventListener('submit', formSubmissionCallback);
}
}, [accessSchedules]);
formSubmissionCallback = onSubmit;

Check warning on line 346 in src/apps/dashboard/routes/users/parentalcontrol.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

Assignments to the 'formSubmissionCallback' variable from inside React Hook useEffect will be lost after each render. To preserve the value over time, store it in a useRef Hook and keep the mutable value in the '.current' property. Otherwise, you can move this variable directly inside useEffect
github-actions[bot] marked this conversation as resolved.
Show resolved Hide resolved
(page.querySelector('.userParentalControlForm') as HTMLFormElement).addEventListener('submit', formSubmissionCallback);
}, [setAllowedTags, setBlockedTags, loadData, userId]);

const optionMaxParentalRating = () => {
let content = '';
Expand All @@ -397,6 +356,21 @@
return content;
};

const removeAllowedTagsCallback = useCallback((tag: string) => {
const newTags = allowedTags.filter(t => t !== tag);
setAllowedTags(newTags);
}, [allowedTags, setAllowedTags]);

const removeBlockedTagsTagsCallback = useCallback((tag: string) => {
const newTags = blockedTags.filter(t => t !== tag);
setBlockedTags(newTags);
}, [blockedTags, setBlockedTags]);

const removeScheduleCallback = useCallback((index: number) => {
const newSchedules = accessSchedules.filter((_e, i) => i != index);
setAccessSchedules(newSchedules);
}, [accessSchedules, setAccessSchedules]);

return (
<Page
id='userParentalControlPage'
Expand Down Expand Up @@ -461,6 +435,7 @@
key={tag}
tag={tag}
tagType='allowedTag'
removeTagCallback={removeAllowedTagsCallback}
/>;
})}
</div>
Expand All @@ -485,6 +460,7 @@
key={tag}
tag={tag}
tagType='blockedTag'
removeTagCallback={removeBlockedTagsTagsCallback}
/>;
})}
</div>
Expand All @@ -508,6 +484,7 @@
DayOfWeek={accessSchedule.DayOfWeek}
StartHour={accessSchedule.StartHour}
EndHour={accessSchedule.EndHour}
removeScheduleCallback={removeScheduleCallback}
/>;
})}
</div>
Expand Down
9 changes: 7 additions & 2 deletions src/components/dashboard/users/AccessScheduleList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { FunctionComponent } from 'react';
import React, { FunctionComponent, useCallback } from 'react';
import datetime from '../../../scripts/datetime';
import globalize from '../../../lib/globalize';
import IconButtonElement from '../../../elements/IconButtonElement';
Expand All @@ -8,6 +8,7 @@ type AccessScheduleListProps = {
DayOfWeek?: string;
StartHour?: number ;
EndHour?: number;
removeScheduleCallback?: (index: number) => void;
};

function getDisplayTime(hours = 0) {
Expand All @@ -21,7 +22,10 @@ function getDisplayTime(hours = 0) {
return datetime.getDisplayTime(new Date(2000, 1, 1, hours, minutes, 0, 0));
}

const AccessScheduleList: FunctionComponent<AccessScheduleListProps> = ({ index, DayOfWeek, StartHour, EndHour }: AccessScheduleListProps) => {
const AccessScheduleList: FunctionComponent<AccessScheduleListProps> = ({ index, DayOfWeek, StartHour, EndHour, removeScheduleCallback }: AccessScheduleListProps) => {
const onClick = useCallback(() => {
index !== undefined && removeScheduleCallback !== undefined && removeScheduleCallback(index);
}, [index, removeScheduleCallback]);
return (
<div
className='liSchedule listItem'
Expand All @@ -43,6 +47,7 @@ const AccessScheduleList: FunctionComponent<AccessScheduleListProps> = ({ index,
title='Delete'
icon='delete'
dataIndex={index}
onClick={onClick}
/>
</div>
);
Expand Down
9 changes: 7 additions & 2 deletions src/components/dashboard/users/TagList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import React, { FunctionComponent } from 'react';
import React, { FunctionComponent, useCallback } from 'react';
import IconButtonElement from '../../../elements/IconButtonElement';

type IProps = {
tag?: string,
tagType?: string;
removeTagCallback?: (tag: string) => void;
};

const TagList: FunctionComponent<IProps> = ({ tag, tagType }: IProps) => {
const TagList: FunctionComponent<IProps> = ({ tag, tagType, removeTagCallback }: IProps) => {
const onClick = useCallback(() => {
tag !== undefined && removeTagCallback !== undefined && removeTagCallback(tag);
}, [tag, removeTagCallback]);
return (
<div className='paperList'>
<div className='listItem'>
Expand All @@ -21,6 +25,7 @@ const TagList: FunctionComponent<IProps> = ({ tag, tagType }: IProps) => {
title='Delete'
icon='delete'
dataTag={tag}
onClick={onClick}
/>
</div>
</div>
Expand Down
4 changes: 3 additions & 1 deletion src/elements/IconButtonElement.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
dataIndex?: string | number;
dataTag?: string | number;
dataProfileid?: string | number;
onClick?: () => void;
};

const createIconButtonElement = ({ is, id, className, title, icon, dataIndex, dataTag, dataProfileid }: IProps) => ({
Expand All @@ -28,9 +29,9 @@
</button>`
});

const IconButtonElement: FunctionComponent<IProps> = ({ is, id, className, title, icon, dataIndex, dataTag, dataProfileid }: IProps) => {
const IconButtonElement: FunctionComponent<IProps> = ({ is, id, className, title, icon, dataIndex, dataTag, dataProfileid, onClick }: IProps) => {
return (
<div

Check failure on line 34 in src/elements/IconButtonElement.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

Visible, non-interactive elements with click handlers must have at least one keyboard listener

Check failure on line 34 in src/elements/IconButtonElement.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️ 

Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element
dangerouslySetInnerHTML={createIconButtonElement({
is: is,
id: id ? `id="${id}"` : '',
Expand All @@ -41,6 +42,7 @@
dataTag: dataTag ? `data-tag="${dataTag}"` : '',
dataProfileid: dataProfileid ? `data-profileid="${dataProfileid}"` : ''
})}
onClick={onClick}
/>
);
};
Expand Down
Loading