Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00
Code Issues Activity

🐛 Fixed missing progress indicator when submitting comments

Browse source 
closes https://linear.app/ghost/issue/PLG-265

- wrapped the async part of `dispatchAction` in a Promise so code that calls it can await the action completion
  - this was a regression introduced a long time ago when we switched to Typescript and React hooks
- added a `setDelay()` method to our `MockedApi` class to make it easier to test interstitial loading states
This commit is contained in:
Kevin Ansfield 2024-11-21 15:59:29 +00:00
parent 368a8eb2f8
commit 0b806cc01b
4 changed files with 41 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
apps/comments-ui/src/App.tsx
View file

@ -70,13 +70,20 @@ const App: React.FC<AppProps> = ({scriptTag}) => {
// This is a bit a ugly hack, but only reliable way to make sure we can get the latest state asynchronously
// without creating infinite rerenders because dispatchAction needs to change on every state change
// So state shouldn't be a dependency of dispatchAction
setState((state) => {
ActionHandler({action, data, state, api, adminApi: state.adminApi!, options}).then((updatedState) => {
setState({...updatedState});
}).catch(console.error); // eslint-disable-line no-console
//
// Wrapped in a Promise so that callers of `dispatchAction` can await the action completion. setState doesn't
// allow for async actions within it's updater function so this is the best option.
return new Promise((resolve) => {
setState((state) => {
ActionHandler({action, data, state, api, adminApi: state.adminApi!, options}).then((updatedState) => {
const newState = {...updatedState};
resolve(newState);
setState(newState);
}).catch(console.error); // eslint-disable-line no-console
// No immediate changes
return {};
// No immediate changes
return {};
});
});
}, [api, options]); // Do not add state or context as a dependency here -> infinite render loop

2
apps/comments-ui/src/components/content/forms/Form.tsx
View file

@ -58,7 +58,7 @@ const FormEditor: React.FC<FormEditorProps> = ({comment, submit, progress, setPr
if (progress === 'sending') {
submitText = null;
buttonIcon = <SpinnerIcon className="h-[24px] w-[24px] fill-white dark:fill-black" />;
buttonIcon = <SpinnerIcon className="h-[24px] w-[24px] fill-white dark:fill-black" data-testid="button-spinner" />;
}
const stopIfFocused = useCallback((event) => {

7
apps/comments-ui/test/e2e/actions.test.ts
View file

@ -177,9 +177,16 @@ test.describe('Actions', async () => {
await page.keyboard.type('This is a reply to a reply');
// give time for spinner to show
mockedApi.setDelay(100);
const submitButton = parentComment.getByTestId('submit-form-button');
await submitButton.click();
// Spinner is shown
await expect(frame.getByTestId('button-spinner')).toBeVisible();
await expect(frame.getByTestId('button-spinner')).not.toBeVisible();
// Comment gets added and has correct contents
await expect(frame.getByTestId('comment-component')).toHaveCount(3);
await expect(frame.getByText('This is a reply to a reply')).toHaveCount(1);

20
apps/comments-ui/test/utils/MockedApi.ts
View file

@ -13,6 +13,7 @@ export class MockedApi {
member: any;
settings: any;
members: any[];
delay: number;
#lastCommentDate = new Date('2021-01-01T00:00:00.000Z');
@ -26,6 +27,11 @@ export class MockedApi {
this.member = member;
this.settings = settings;
this.members = [];
this.delay = 0;
}
setDelay(delay: number) {
this.delay = delay;
}
addComment(overrides: any = {}) {
@ -220,8 +226,15 @@ export class MockedApi {
};
}
async #delayResponse() {
await new Promise((resolve) => {
(setTimeout(resolve, this.delay));
});
}
async listen({page, path}: {page: any, path: string}) {
await page.route(`${path}/members/api/member/`, async (route) => {
await this.#delayResponse();
if (!this.member) {
return await route.fulfill({
status: 401,
@ -244,6 +257,7 @@ export class MockedApi {
});
await page.route(`${path}/members/api/comments/*`, async (route) => {
await this.#delayResponse();
const payload = JSON.parse(route.request().postData());
this.#lastCommentDate = new Date();
@ -262,6 +276,7 @@ export class MockedApi {
});
await page.route(`${path}/members/api/comments/post/*/*`, async (route) => {
await this.#delayResponse();
const url = new URL(route.request().url());
const p = parseInt(url.searchParams.get('page') ?? '1');
@ -281,6 +296,7 @@ export class MockedApi {
// LIKE a single comment
await page.route(`${path}/members/api/comments/*/like/`, async (route) => {
await this.#delayResponse();
const url = new URL(route.request().url());
const commentId = url.pathname.split('/').reverse()[2];
@ -315,6 +331,7 @@ export class MockedApi {
// GET a single comment
await page.route(`${path}/members/api/comments/*/`, async (route) => {
await this.#delayResponse();
const url = new URL(route.request().url());
const commentId = url.pathname.split('/').reverse()[1];
@ -330,6 +347,7 @@ export class MockedApi {
});
await page.route(`${path}/members/api/comments/*/replies/*`, async (route) => {
await this.#delayResponse();
const url = new URL(route.request().url());
const limit = parseInt(url.searchParams.get('limit') ?? '5');
@ -347,6 +365,7 @@ export class MockedApi {
});
await page.route(`${path}/members/api/comments/counts/*`, async (route) => {
await this.#delayResponse();
await route.fulfill({
status: 200,
body: JSON.stringify(
@ -358,6 +377,7 @@ export class MockedApi {
// get settings from content api
await page.route(`${path}/settings/*`, async (route) => {
await this.#delayResponse();
await route.fulfill({
status: 200,
body: JSON.stringify(this.settings)