Skip to content

Commit 6d5f0a3

Browse files
raclimraclim
authored
Merge branch 'develop' into Fixing-Inconsistent-Behavior-of-Play-Sketch-Button
2 parents e1ea666 + 64a5f29 commit 6d5f0a3

72 files changed

Lines changed: 20613 additions & 26264 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/PULL_REQUEST_TEMPLATE.md‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
1-
Fixes #issue-number
1+
### Context or Issue Number:
2+
<!-- Please add the issue this relates to if one is available. Otherwise please describe the current state of the codebase and what this PR attempts to fix. -->
23

3-
Changes:
4+
### Demo:
5+
<!-- Please add a screenshot for UI related changes -->
46

5-
I have verified that this pull request:
7+
### Changes:
8+
<!-- Summarise your changes -->
9+
10+
### I have verified that this pull request:
611

712
* [ ] has no linting errors (`npm run lint`)
813
* [ ] has no test errors (`npm run test`)

‎client/common/Tooltip.test.tsx‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import React from 'react';
2+
import userEvent from '@testing-library/user-event';
3+
import { render, screen } from '../test-utils';
4+
import { Tooltip } from './Tooltip';
5+
6+
describe('Tooltip', () => {
7+
it('renders the child element', () => {
8+
render(
9+
<Tooltip content="This is a tooltip">
10+
<button>Hover me</button>
11+
</Tooltip>
12+
);
13+
expect(screen.getByRole('button')).toBeInTheDocument();
14+
expect(screen.getByText('Hover me')).toBeInTheDocument();
15+
});
16+
17+
it('does not show the tooltip when the user is not hovering over the element', () => {
18+
render(
19+
<Tooltip content="Tooltip text">
20+
<button>Button</button>
21+
</Tooltip>
22+
);
23+
24+
const button = screen.getByRole('button');
25+
expect(button).toBeInTheDocument();
26+
expect(button).not.toHaveClass('tooltipped-visible');
27+
});
28+
29+
it('shows the tooltip if the user hovers over the element', async () => {
30+
const user = userEvent.setup();
31+
render(
32+
<Tooltip content="Tooltip text">
33+
<button>Button</button>
34+
</Tooltip>
35+
);
36+
37+
const button = screen.getByRole('button');
38+
await user.hover(button);
39+
40+
expect(button).toHaveClass('tooltipped');
41+
expect(button).toHaveAttribute('aria-label', 'Tooltip text');
42+
});
43+
44+
it('adds the aria-label with tooltip content to the child element', () => {
45+
render(
46+
<Tooltip content="Save your changes">
47+
<button>Save</button>
48+
</Tooltip>
49+
);
50+
51+
const button = screen.getByRole('button');
52+
expect(button).toHaveAttribute('aria-label', 'Save your changes');
53+
});
54+
55+
it('applies tooltipped-no-delay class when noDelay is true', () => {
56+
render(
57+
<Tooltip content="No delay tooltip" noDelay>
58+
<button>Button</button>
59+
</Tooltip>
60+
);
61+
62+
const button = screen.getByRole('button');
63+
expect(button).toHaveClass('tooltipped-no-delay');
64+
});
65+
66+
it('does not apply tooltipped-no-delay class when noDelay is false', () => {
67+
render(
68+
<Tooltip content="Normal tooltip" noDelay={false}>
69+
<button>Button</button>
70+
</Tooltip>
71+
);
72+
73+
const button = screen.getByRole('button');
74+
expect(button).not.toHaveClass('tooltipped-no-delay');
75+
});
76+
77+
it('preserves existing className on the child element', () => {
78+
render(
79+
<Tooltip content="Tooltip">
80+
<button className="custom-class">Button</button>
81+
</Tooltip>
82+
);
83+
84+
const button = screen.getByRole('button');
85+
expect(button).toHaveClass('custom-class');
86+
expect(button).toHaveClass('tooltipped');
87+
});
88+
89+
it('wraps the child in a tooltip-wrapper span', () => {
90+
const { container } = render(
91+
<Tooltip content="Tooltip">
92+
<button>Button</button>
93+
</Tooltip>
94+
);
95+
96+
const wrapper = container.querySelector('.tooltip-wrapper');
97+
expect(wrapper).toBeInTheDocument();
98+
expect(wrapper?.tagName.toLowerCase()).toBe('span');
99+
});
100+
});

‎client/common/Tooltip.tsx‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import React, { ReactElement, useMemo } from 'react';
2+
3+
export type TooltipProps = {
4+
content: string;
5+
noDelay?: boolean;
6+
children: ReactElement;
7+
};
8+
9+
export function Tooltip({ content, noDelay = false, children }: TooltipProps) {
10+
const tooltipClasses = useMemo(() => {
11+
const existingClassName = children.props?.className || '';
12+
return [
13+
existingClassName,
14+
'tooltipped',
15+
'tooltipped-n',
16+
noDelay && 'tooltipped-no-delay'
17+
]
18+
.filter(Boolean)
19+
.join(' ');
20+
}, [children.props?.className, noDelay]);
21+
22+
const childProps = useMemo(
23+
() => ({
24+
'aria-label': content,
25+
className: tooltipClasses
26+
}),
27+
[content, tooltipClasses]
28+
);
29+
30+
return (
31+
<span className="tooltip-wrapper">
32+
{React.cloneElement(children, childProps)}
33+
</span>
34+
);
35+
}

‎client/components/Menubar/MenubarItem.tsx‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React, { useEffect, useContext, useRef } from 'react';
22
import { MenubarContext, SubmenuContext, ParentMenuContext } from './contexts';
33
import { ButtonOrLink, ButtonOrLinkProps } from '../../common/ButtonOrLink';
4+
import { Tooltip, TooltipProps } from '../../common/Tooltip';
45

56
export enum MenubarItemRole {
67
MENU_ITEM = 'menuitem',
@@ -13,6 +14,7 @@ export interface MenubarItemProps extends Omit<ButtonOrLinkProps, 'role'> {
1314
*/
1415
role?: MenubarItemRole;
1516
selected?: boolean;
17+
tooltipContent?: TooltipProps['content'];
1618
}
1719

1820
/**
@@ -54,6 +56,7 @@ export function MenubarItem({
5456
role: customRole = MenubarItemRole.MENU_ITEM,
5557
isDisabled = false,
5658
selected = false,
59+
tooltipContent,
5760
...rest
5861
}: MenubarItemProps) {
5962
const { createMenuItemHandlers, hasFocus } = useContext(MenubarContext);
@@ -94,15 +97,29 @@ export function MenubarItem({
9497
ref={menuItemRef}
9598
onMouseEnter={handleMouseEnter}
9699
>
97-
<ButtonOrLink
98-
{...rest}
99-
{...handlers}
100-
{...ariaSelected}
101-
role={role}
102-
tabIndex={-1}
103-
id={id}
104-
isDisabled={isDisabled}
105-
/>
100+
{tooltipContent ? (
101+
<Tooltip content={tooltipContent}>
102+
<ButtonOrLink
103+
{...rest}
104+
{...handlers}
105+
{...ariaSelected}
106+
role={role}
107+
tabIndex={-1}
108+
id={id}
109+
isDisabled={isDisabled}
110+
/>
111+
</Tooltip>
112+
) : (
113+
<ButtonOrLink
114+
{...rest}
115+
{...handlers}
116+
{...ariaSelected}
117+
role={role}
118+
tabIndex={-1}
119+
id={id}
120+
isDisabled={isDisabled}
121+
/>
122+
)}
106123
</li>
107124
);
108125
}

‎client/constants.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export const RESET_PROJECT = 'RESET_PROJECT';
3333

3434
export const SET_PROJECT = 'SET_PROJECT';
3535
export const SET_PROJECTS = 'SET_PROJECTS';
36+
export const SET_PROJECTS_FOR_COLLECTION_LIST =
37+
'SET_PROJECTS_FOR_COLLECTION_LIST';
3638

3739
export const SET_COLLECTIONS = 'SET_COLLECTIONS';
3840
export const CREATE_COLLECTION = 'CREATED_COLLECTION';

‎client/modules/About/pages/About.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ export const About = () => {
167167
<a
168168
href="https://github.com/processing/p5.js-web-editor/releases"
169169
target="_blank"
170-
rel="noreferrer"
170+
rel="noopener noreferrer"
171171
>
172172
{t('About.WebEditor')}: <span>v{packageData?.version}</span>
173173
</a>
@@ -176,7 +176,7 @@ export const About = () => {
176176
<a
177177
href="https://github.com/processing/p5.js/releases"
178178
target="_blank"
179-
rel="noreferrer"
179+
rel="noopener noreferrer"
180180
>
181181
p5.js: <span>v{p5version}</span>
182182
</a>

‎client/modules/App/components/Overlay.tsx‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React, { useCallback, useRef } from 'react';
2-
import MediaQuery from 'react-responsive';
2+
import { useMediaQuery } from 'react-responsive';
33
import { useSelector } from 'react-redux';
44
import { useHistory } from 'react-router-dom';
55
import { useTranslation } from 'react-i18next';
@@ -9,8 +9,8 @@ import type { RootState } from '../../../reducers';
99
import ExitIcon from '../../../images/exit.svg';
1010

1111
type OverlayProps = {
12-
children?: React.ReactElement;
13-
actions?: React.ReactElement;
12+
children?: React.ReactNode;
13+
actions?: React.ReactNode;
1414
closeOverlay?: () => void;
1515
title?: string;
1616
ariaLabel?: string;
@@ -35,6 +35,9 @@ export const Overlay = ({
3535

3636
const browserHistory = useHistory();
3737

38+
const isDesktop = useMediaQuery({ minWidth: 770 });
39+
const isMobile = useMediaQuery({ maxWidth: 769 });
40+
3841
const close = useCallback(() => {
3942
const node = ref.current;
4043
if (!node) return;
@@ -66,7 +69,7 @@ export const Overlay = ({
6669
<header className="overlay__header">
6770
<h2 className="overlay__title">{title}</h2>
6871
<div className="overlay__actions">
69-
<MediaQuery minWidth={770}>{actions}</MediaQuery>
72+
{isDesktop && actions}
7073
<button
7174
className="overlay__close-button"
7275
onClick={close}
@@ -76,11 +79,9 @@ export const Overlay = ({
7679
</button>
7780
</div>
7881
</header>
79-
<MediaQuery maxWidth={769}>
80-
{actions && (
81-
<div className="overlay__actions-mobile">{actions}</div>
82-
)}
83-
</MediaQuery>
82+
{isMobile && actions && (
83+
<div className="overlay__actions-mobile">{actions}</div>
84+
)}
8485
{children}
8586
</section>
8687
</div>

‎client/modules/IDE/actions/assets.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ export function getAssets() {
3030
export function deleteAssetRequest(assetKey) {
3131
return async (dispatch) => {
3232
try {
33-
await apiClient.delete(`/S3/${assetKey}`);
33+
const path = assetKey.split('/').pop();
34+
await apiClient.delete(
35+
`/S3/delete?objectKey=${encodeURIComponent(path)}`
36+
);
3437
dispatch(deleteAsset(assetKey));
3538
} catch (error) {
3639
dispatch({

‎client/modules/IDE/actions/project.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ export function changeProjectName(id, newName) {
386386
}
387387

388388
export function deleteProject(id) {
389-
return (dispatch, getState) => {
389+
return (dispatch, getState) =>
390390
apiClient
391391
.delete(`/projects/${id}`)
392392
.then(() => {
@@ -411,7 +411,6 @@ export function deleteProject(id) {
411411
});
412412
}
413413
});
414-
};
415414
}
416415
export function changeVisibility(projectId, projectName, visibility, t) {
417416
return (dispatch, getState) => {

0 commit comments

Comments
 (0)