From 39d0ddea5d634754131d01e621d5271cf36186cd Mon Sep 17 00:00:00 2001
From: lauslim12 <31909304+lauslim12@users.noreply.github.com>
Date: Sun, 7 Jan 2024 12:50:48 +0900
Subject: [PATCH 1/3] Use `encodeURIComponent` and `decodeURIComponent` to
prevent errors
Reference: https://github.com/lauslim12/speednote/issues/36
---
app/Editor.tsx | 80 +++++++++++++++++++++++++++-----------------------
1 file changed, 44 insertions(+), 36 deletions(-)
diff --git a/app/Editor.tsx b/app/Editor.tsx
index 2a29107..5eed268 100644
--- a/app/Editor.tsx
+++ b/app/Editor.tsx
@@ -36,39 +36,44 @@ type SharedNoteProps = {
content: string;
};
-const SharedNote = ({ title, content }: SharedNoteProps) => {
- return (
- <>
-
-
-
-
-
-
-
-
-
-
- Return to your note
-
-
- >
- );
-};
+/**
+ * In this component, the `value` of the `title` and `content` has to be inserted into
+ * `decodeURIComponent` to prevent errors when sharing notes with Kanji characters, or any
+ * other characters outside of the `Latin1` range.
+ *
+ * {@link https://github.com/lauslim12/speednote/issues/36}
+ */
+const SharedNote = ({ title, content }: SharedNoteProps) => (
+ <>
+
+
+
+
+
+
+
+
+
+
+ Return to your note
+
+
+ >
+);
const NoteEditorRoot = () => {
const storage = useStorage();
@@ -226,14 +231,17 @@ const ExternalNoteAction = ({ onSave }: ActionBaseProps) => {
// Save the note initially, so we're using the final state.
onSave();
- // Set new query parameters.
+ // Set new query parameters. Encode as URI component to prevent
+ // failure when sharing Kanji characters or any other characters
+ // that exist outside of the Latin1 range.
+ // Reference: https://github.com/lauslim12/speednote/issues/36.
const newSearchParams = new URLSearchParams();
if (title !== '') {
- newSearchParams.set('title', window.btoa(title));
+ newSearchParams.set('title', window.btoa(encodeURIComponent(title)));
}
if (content !== '') {
- newSearchParams.set('content', window.btoa(content));
+ newSearchParams.set('content', window.btoa(encodeURIComponent(content)));
}
// Copy the URL the user's clipboard. I know that the `writeText` is supposed
From fbf901f371deeec97649da44a1ba1349aa9943d6 Mon Sep 17 00:00:00 2001
From: lauslim12 <31909304+lauslim12@users.noreply.github.com>
Date: Sun, 7 Jan 2024 12:51:28 +0900
Subject: [PATCH 2/3] Use parameterized test to test sharing a note with Kanji
characters
---
__tests__/Speednote.test.tsx | 104 ++++++++++-----
e2e/Speednote.test.tsx | 250 ++++++++++++++++++++---------------
2 files changed, 219 insertions(+), 135 deletions(-)
diff --git a/__tests__/Speednote.test.tsx b/__tests__/Speednote.test.tsx
index c55b274..0a0d4f8 100644
--- a/__tests__/Speednote.test.tsx
+++ b/__tests__/Speednote.test.tsx
@@ -266,37 +266,67 @@ test('able to freeze notes and unfreeze them', async () => {
expect(content).toHaveValue('Hi there, I just wanted to type this note.');
});
-test('able to copy shared note properly', async () => {
- // Because it's important to ensure that the title and the content is encoded properly,
- // I decided to spy on this function to make sure that it doesn't do anything unexpected.
- const mockWriteText = jest
- .spyOn(window.navigator.clipboard, 'writeText')
- .mockImplementation();
-
- // Render the app with our new browser context.
- const { user } = renderWithProviders();
-
- // Write something on the inputs.
- const { title, content } = assertEditor();
- await user.type(title, 'Income');
- expect(title).toHaveValue('Income');
- await user.type(content, 'I finished a project and received 5000 JPY.');
- expect(content).toHaveValue('I finished a project and received 5000 JPY.');
-
- // Click on the `Share note` button.
- const expectedEncodedTitle = encodeURIComponent('SW5jb21l');
- const expectedEncodedContent = encodeURIComponent(
- 'SSBmaW5pc2hlZCBhIHByb2plY3QgYW5kIHJlY2VpdmVkIDUwMDAgSlBZLg=='
- );
- const shareNoteButton = screen.getByRole('button', {
- name: 'Copy/share note link',
- });
- await user.click(shareNoteButton);
-
- const expectedUrl = `${window.location.href}?title=${expectedEncodedTitle}&content=${expectedEncodedContent}`;
- expect(mockWriteText).toHaveBeenCalledTimes(1);
- expect(mockWriteText).toHaveBeenCalledWith(expectedUrl);
-});
+test.each([
+ {
+ constraint: 'with normal characters',
+ inputTitle: 'Income',
+ inputContent: 'I finished a project and received 5000 JPY.',
+ expectedEncodedTitle: 'SW5jb21l',
+ expectedEncodedContent:
+ 'SSUyMGZpbmlzaGVkJTIwYSUyMHByb2plY3QlMjBhbmQlMjByZWNlaXZlZCUyMDUwMDAlMjBKUFku',
+ },
+ {
+ constraint: 'with japanese characters',
+ inputTitle: 'ノートです!',
+ inputContent: 'このノートは見本です。',
+ expectedEncodedTitle:
+ 'JUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUE3JUUzJTgxJTk5JUVGJUJDJTgx',
+ expectedEncodedContent:
+ 'JUUzJTgxJTkzJUUzJTgxJUFFJUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUFGJUU4JUE2JThCJUU2JTlDJUFDJUUzJTgxJUE3JUUzJTgxJTk5JUUzJTgwJTgy',
+ },
+ {
+ constraint: 'with chinese characters',
+ inputTitle: '你好!',
+ inputContent: '这是一张纸条样本。',
+ expectedEncodedTitle: 'JUU0JUJEJUEwJUU1JUE1JUJEJUVGJUJDJTgx',
+ expectedEncodedContent:
+ 'JUU4JUJGJTk5JUU2JTk4JUFGJUU0JUI4JTgwJUU1JUJDJUEwJUU3JUJBJUI4JUU2JTlEJUExJUU2JUEwJUI3JUU2JTlDJUFDJUUzJTgwJTgy',
+ },
+])(
+ 'able to copy shared note properly $constraint',
+ async ({
+ inputTitle,
+ inputContent,
+ expectedEncodedTitle,
+ expectedEncodedContent,
+ }) => {
+ // Because it's important to ensure that the title and the content is encoded properly,
+ // I decided to spy on this function to make sure that it doesn't do anything unexpected.
+ const mockWriteText = jest
+ .spyOn(window.navigator.clipboard, 'writeText')
+ .mockImplementation();
+
+ // Render the app with our new browser context.
+ const { user } = renderWithProviders();
+
+ // Write something on the inputs.
+ const { title, content } = assertEditor();
+ await user.type(title, inputTitle);
+ expect(title).toHaveValue(inputTitle);
+ await user.type(content, inputContent);
+ expect(content).toHaveValue(inputContent);
+
+ // Click on the `Share note` button.
+ const shareNoteButton = screen.getByRole('button', {
+ name: 'Copy/share note link',
+ });
+ await user.click(shareNoteButton);
+
+ const expectedUrl = `${window.location.href}?title=${expectedEncodedTitle}&content=${expectedEncodedContent}`;
+ expect(mockWriteText).toHaveBeenCalledTimes(1);
+ expect(mockWriteText).toHaveBeenCalledWith(expectedUrl);
+ }
+);
test('able to see shared URL properly', async () => {
const startUrl = `${window.location.href}?title=SW5jb21l&content=SSBmaW5pc2hlZCBhIHByb2plY3QgYW5kIHJlY2VpdmVkIDUwMDAgSlBZLg%3D%3D`;
@@ -347,6 +377,18 @@ test.each([
expectedTitle: 'No title in the shared note',
expectedContent: 'Beautiful Trauma',
},
+ {
+ name: 'valid title and content, japanese characters',
+ url: '?title=JUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUE3JUUzJTgxJTk5JUVGJUJDJTgx&content=JUUzJTgxJTkzJUUzJTgxJUFFJUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUFGJUU4JUE2JThCJUU2JTlDJUFDJUUzJTgxJUE3JUUzJTgxJTk5JUUzJTgwJTgy',
+ expectedTitle: 'ノートです!',
+ expectedContent: 'このノートは見本です。',
+ },
+ {
+ name: 'valid title and content, chinese characters',
+ url: '?title=JUU0JUJEJUEwJUU1JUE1JUJEJUVGJUJDJTgx&content=JUU4JUJGJTk5JUU2JTk4JUFGJUU0JUI4JTgwJUU1JUJDJUEwJUU3JUJBJUI4JUU2JTlEJUExJUU2JUEwJUI3JUU2JTlDJUFDJUUzJTgwJTgy',
+ expectedTitle: '你好!',
+ expectedContent: '这是一张纸条样本。',
+ },
{
name: 'invalid title and content',
url: '?title=xxx&content=yyy',
diff --git a/e2e/Speednote.test.tsx b/e2e/Speednote.test.tsx
index fb37c0c..68e44af 100644
--- a/e2e/Speednote.test.tsx
+++ b/e2e/Speednote.test.tsx
@@ -229,115 +229,145 @@ test('able to freeze notes and unfreeze them', async ({ page }) => {
);
});
-test('able to copy and see a shared note properly', async () => {
- // We need to use a new browser context to allow permission to write to clipboard in the headless Playwright test.
- const browser = await chromium.launch();
- const context = await browser.newContext({
- permissions: ['clipboard-write', 'clipboard-read'],
- });
- const page = await context.newPage();
-
- // Render the app with our new browser context.
- await renderPage(page);
-
- // Write something on the inputs.
- const { title, content } = await getAndAssertEditor(page);
- const freezeNoteButton = page.getByRole('button', { name: 'Freeze note' });
- await expect(freezeNoteButton).toBeVisible();
- await title.fill('Income');
- await expect(title).toHaveValue('Income');
- await content.fill('I finished a project and received 5000 JPY.');
- await expect(content).toHaveValue(
- 'I finished a project and received 5000 JPY.'
- );
-
- // Click on the `Share note` button.
- const shareNoteButton = page.getByRole('button', { name: 'Share note' });
- await shareNoteButton.click();
-
- // Slightly testing implementation details, make sure that the shared URL is correct. We don't
- // care about the leading parts, we just want to make sure that the URL query params are correct.
- const expectedEncodedTitle = encodeURIComponent('SW5jb21l');
- const expectedEncodedContent = encodeURIComponent(
- 'SSBmaW5pc2hlZCBhIHByb2plY3QgYW5kIHJlY2VpdmVkIDUwMDAgSlBZLg=='
- );
- const clipboardText = await page.evaluate('navigator.clipboard.readText()');
- const expectedUrl = `?title=${expectedEncodedTitle}&content=${expectedEncodedContent}`;
- expect(clipboardText).toContain(expectedUrl);
-
- // Summon a new page based on the copied URL query parameters.
- const newPage = await context.newPage();
- await renderPage(newPage, clipboardText as string);
+const copyAndSeeSharedNoteTestCases = [
+ {
+ constraint: 'with normal characters',
+ inputTitle: 'Income',
+ inputContent: 'I finished a project and received 5000 JPY.',
+ expectedEncodedTitle: 'SW5jb21l',
+ expectedEncodedContent:
+ 'SSUyMGZpbmlzaGVkJTIwYSUyMHByb2plY3QlMjBhbmQlMjByZWNlaXZlZCUyMDUwMDAlMjBKUFku',
+ },
+ {
+ constraint: 'with japanese characters',
+ inputTitle: 'ノートです!',
+ inputContent: 'このノートは見本です。',
+ expectedEncodedTitle:
+ 'JUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUE3JUUzJTgxJTk5JUVGJUJDJTgx',
+ expectedEncodedContent:
+ 'JUUzJTgxJTkzJUUzJTgxJUFFJUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUFGJUU4JUE2JThCJUU2JTlDJUFDJUUzJTgxJUE3JUUzJTgxJTk5JUUzJTgwJTgy',
+ },
+ {
+ constraint: 'with chinese characters',
+ inputTitle: '你好!',
+ inputContent: '这是一张纸条样本。',
+ expectedEncodedTitle: 'JUU0JUJEJUEwJUU1JUE1JUJEJUVGJUJDJTgx',
+ expectedEncodedContent:
+ 'JUU4JUJGJTk5JUU2JTk4JUFGJUU0JUI4JTgwJUU1JUJDJUEwJUU3JUJBJUI4JUU2JTlEJUExJUU2JUEwJUI3JUU2JTlDJUFDJUUzJTgwJTgy',
+ },
+];
- const { title: newTitle, content: newContent } = await getAndAssertEditor(
- newPage
- );
- await expect(newTitle).toHaveValue('Income');
- await expect(newContent).toHaveValue(
- 'I finished a project and received 5000 JPY.'
- );
+for (const {
+ constraint,
+ inputTitle,
+ inputContent,
+ expectedEncodedTitle,
+ expectedEncodedContent,
+} of copyAndSeeSharedNoteTestCases) {
+ test(`able to copy and see a shared note properly ${constraint}`, async () => {
+ // We need to use a new browser context to allow permission to write to clipboard in the headless Playwright test.
+ const browser = await chromium.launch();
+ const context = await browser.newContext({
+ permissions: ['clipboard-write', 'clipboard-read'],
+ });
+ const page = await context.newPage();
- // Freeze note button should not be here, and all of the inputs should be read only.
- const newFreezeNoteButton = newPage.getByRole('button', {
- name: 'Freeze note',
- });
- await expect(newFreezeNoteButton).not.toBeVisible();
- await expect(newTitle).not.toBeEditable();
- await expect(newContent).not.toBeEditable();
+ // Render the app with our new browser context.
+ await renderPage(page);
- // We should not be able to re-share the note to another person. The button should be hidden
- // as it is very confusing (which one to share? Our note or the shared note? Communicating the message
- // is difficult, so it's better to make it explicit and just do not make the button visible on a shared note).
- await expect(
- newPage.getByRole('button', { name: 'Share note' })
- ).not.toBeVisible();
-
- // Edge-case: update the different page, the local storage should be synced - after the user decided to return
- // to the normal note, the user should see the normal note.
- await title.clear();
- await title.fill('Expense');
- await expect(title).toHaveValue('Expense');
- await content.clear();
- await content.fill('Hi there!');
- await expect(content).toHaveValue('Hi there!');
- await freezeNoteButton.click();
- await expect(title).not.toBeEditable();
- await expect(content).not.toBeEditable();
+ // Write something on the inputs.
+ const { title, content } = await getAndAssertEditor(page);
+ const freezeNoteButton = page.getByRole('button', { name: 'Freeze note' });
+ await expect(freezeNoteButton).toBeVisible();
+ await title.fill(inputTitle);
+ await expect(title).toHaveValue(inputTitle);
+ await content.fill(inputContent);
+ await expect(content).toHaveValue(inputContent);
+
+ // Click on the `Share note` button.
+ const shareNoteButton = page.getByRole('button', { name: 'Share note' });
+ await shareNoteButton.click();
+
+ // Slightly testing implementation details, make sure that the shared URL is correct. We don't
+ // care about the leading parts, we just want to make sure that the URL query params are correct.
+ const clipboardText = await page.evaluate('navigator.clipboard.readText()');
+ const expectedUrl = `?title=${expectedEncodedTitle}&content=${expectedEncodedContent}`;
+ expect(clipboardText).toContain(expectedUrl);
+
+ // Summon a new page based on the copied URL query parameters.
+ const newPage = await context.newPage();
+ await renderPage(newPage, clipboardText as string);
+
+ const { title: newTitle, content: newContent } = await getAndAssertEditor(
+ newPage
+ );
+ await expect(newTitle).toHaveValue(inputTitle);
+ await expect(newContent).toHaveValue(inputContent);
+
+ // Freeze note button should not be here, and all of the inputs should be read only.
+ const newFreezeNoteButton = newPage.getByRole('button', {
+ name: 'Freeze note',
+ });
+ await expect(newFreezeNoteButton).not.toBeVisible();
+ await expect(newTitle).not.toBeEditable();
+ await expect(newContent).not.toBeEditable();
+
+ // We should not be able to re-share the note to another person. The button should be hidden
+ // as it is very confusing (which one to share? Our note or the shared note? Communicating the message
+ // is difficult, so it's better to make it explicit and just do not make the button visible on a shared note).
+ await expect(
+ newPage.getByRole('button', { name: 'Share note' })
+ ).not.toBeVisible();
+
+ // Edge-case: update the different page, the local storage should be synced - after the user decided to return
+ // to the normal note, the user should see the normal note.
+ await title.clear();
+ await title.fill('Expense');
+ await expect(title).toHaveValue('Expense');
+ await content.clear();
+ await content.fill('Hi there!');
+ await expect(content).toHaveValue('Hi there!');
+ await freezeNoteButton.click();
+ await expect(title).not.toBeEditable();
+ await expect(content).not.toBeEditable();
- // Return to the normal, ensure that components and the freeze button are here.
- const returnButton = newPage.getByRole('link', {
- name: 'Return to your note',
- });
- await expect(returnButton).toBeVisible();
- await returnButton.click();
-
- // Should re-render again, validate again that all have rendered successfully.
- await newPage.waitForURL('/');
- await expect(newPage).toHaveURL('/');
-
- // Edge-case: make sure that once we get back to the normal page, the local storage is synced properly and the
- // change is using the latest one after we have updated the inputs after opening the link from the clipboard.
- const rerenderedTitle = newPage.getByRole('textbox', { name: 'Note title' });
- const rerenderedContent = newPage.getByRole('textbox', {
- name: 'Note content',
+ // Return to the normal, ensure that components and the freeze button are here.
+ const returnButton = newPage.getByRole('link', {
+ name: 'Return to your note',
+ });
+ await expect(returnButton).toBeVisible();
+ await returnButton.click();
+
+ // Should re-render again, validate again that all have rendered successfully.
+ await newPage.waitForURL('/');
+ await expect(newPage).toHaveURL('/');
+
+ // Edge-case: make sure that once we get back to the normal page, the local storage is synced properly and the
+ // change is using the latest one after we have updated the inputs after opening the link from the clipboard.
+ const rerenderedTitle = newPage.getByRole('textbox', {
+ name: 'Note title',
+ });
+ const rerenderedContent = newPage.getByRole('textbox', {
+ name: 'Note content',
+ });
+ const secondUpdatedFrozenButton = newPage.getByRole('button', {
+ name: 'Freeze note',
+ });
+
+ await expect(rerenderedTitle).toHaveValue('Expense');
+ await expect(rerenderedContent).toHaveValue('Hi there!');
+ await expect(secondUpdatedFrozenButton).toBeVisible();
+
+ // Verify that the `Share note` button is visible again after returning to the `/` page.
+ await expect(
+ newPage.getByRole('button', { name: 'Share note' })
+ ).toBeVisible();
+
+ // Close all sessions.
+ await context.close();
+ await browser.close();
});
- const secondUpdatedFrozenButton = newPage.getByRole('button', {
- name: 'Freeze note',
- });
-
- await expect(rerenderedTitle).toHaveValue('Expense');
- await expect(rerenderedContent).toHaveValue('Hi there!');
- await expect(secondUpdatedFrozenButton).toBeVisible();
-
- // Verify that the `Share note` button is visible again after returning to the `/` page.
- await expect(
- newPage.getByRole('button', { name: 'Share note' })
- ).toBeVisible();
-
- // Close all sessions.
- await context.close();
- await browser.close();
-});
+}
// Playwright doesn't support Jest's `test.each`, so we have to use this looping workaround.
const invalidFormatUrlTestCases = [
@@ -353,6 +383,18 @@ const invalidFormatUrlTestCases = [
expectedTitle: 'No title in the shared note',
expectedContent: 'Beautiful Trauma',
},
+ {
+ name: 'valid title and content, japanese characters',
+ url: '?title=JUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUE3JUUzJTgxJTk5JUVGJUJDJTgx&content=JUUzJTgxJTkzJUUzJTgxJUFFJUUzJTgzJThFJUUzJTgzJUJDJUUzJTgzJTg4JUUzJTgxJUFGJUU4JUE2JThCJUU2JTlDJUFDJUUzJTgxJUE3JUUzJTgxJTk5JUUzJTgwJTgy',
+ expectedTitle: 'ノートです!',
+ expectedContent: 'このノートは見本です。',
+ },
+ {
+ name: 'valid title and content, chinese characters',
+ url: '?title=JUU0JUJEJUEwJUU1JUE1JUJEJUVGJUJDJTgx&content=JUU4JUJGJTk5JUU2JTk4JUFGJUU0JUI4JTgwJUU1JUJDJUEwJUU3JUJBJUI4JUU2JTlEJUExJUU2JUEwJUI3JUU2JTlDJUFDJUUzJTgwJTgy',
+ expectedTitle: '你好!',
+ expectedContent: '这是一张纸条样本。',
+ },
{
name: 'invalid title and content',
url: '?title=xxx&content=yyy',
From 5abf8397caf1fca89aaad0529fe5257e40dc8f49 Mon Sep 17 00:00:00 2001
From: lauslim12 <31909304+lauslim12@users.noreply.github.com>
Date: Sun, 7 Jan 2024 12:51:49 +0900
Subject: [PATCH 3/3] Add a note about `404 Not Found` end-to-end tests failure
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/README.md b/README.md
index 1b9d3f7..b5809c8 100644
--- a/README.md
+++ b/README.md
@@ -82,7 +82,7 @@ There are no dependencies (environment variables or the like), this is a standal
## Tests
-Because this project is still small, we can achieve a relatively high enough code coverage. There are two test libraries used in this project: React Testing Library and Playwright.
+Because this project is still small, we can achieve a relatively high enough code coverage. There are two test libraries used in this project: React Testing Library and Playwright. For some reason, if you run the Playwright tests locally without building (using the production version), the `404 Not Found` tests will fail. It is possible that this is caused by the development build.
```bash
# To run RTL tests, do the commands below: