Skip to content

Commit

Permalink
Merge pull request #37 from lauslim12/36-fix-latin1-characters-sharing
Browse files Browse the repository at this point in the history
Fix `Latin1` Encoding Errors, Support Kanji Characters in Note Sharing
  • Loading branch information
lauslim12 committed Jan 7, 2024
2 parents 5992f93 + 5abf839 commit 4217a63
Show file tree
Hide file tree
Showing 4 changed files with 264 additions and 172 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
104 changes: 73 additions & 31 deletions __tests__/Speednote.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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`;
Expand Down Expand Up @@ -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',
Expand Down
80 changes: 44 additions & 36 deletions app/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,39 +36,44 @@ type SharedNoteProps = {
content: string;
};

const SharedNote = ({ title, content }: SharedNoteProps) => {
return (
<>
<section className={styles.section}>
<Input
id="note-title"
aria-label="Note title"
type="title"
placeholder={title}
value={title}
readOnly
/>
</section>

<section className={styles.section}>
<Input
id="note-content"
aria-label="Note content"
type="content"
placeholder={content}
value={content}
readOnly
/>
</section>

<section className={styles.section}>
<Link type="internal" href="/">
Return to your note
</Link>
</section>
</>
);
};
/**
* 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) => (
<>
<section className={styles.section}>
<Input
id="note-title"
aria-label="Note title"
type="title"
placeholder={title}
value={decodeURIComponent(title)}
readOnly
/>
</section>

<section className={styles.section}>
<Input
id="note-content"
aria-label="Note content"
type="content"
placeholder={content}
value={decodeURIComponent(content)}
readOnly
/>
</section>

<section className={styles.section}>
<Link type="internal" href="/">
Return to your note
</Link>
</section>
</>
);

const NoteEditorRoot = () => {
const storage = useStorage();
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 4217a63

Please sign in to comment.