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.

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

Desktop: Fixes #10077 - Special characters in notebooks and tags are not sorted alphabetically #10085

Merged
merged 13 commits into from
Mar 20, 2024
Merged
8 changes: 5 additions & 3 deletions packages/app-desktop/gui/TagList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { useMemo } from 'react';
import { AppState } from '../app.reducer';
import TagItem from './TagItem';
import { getCollator, getCollatorLocale } from '@joplin/lib/models/utils/getCollator';

const { connect } = require('react-redux');
const { themeStyle } = require('@joplin/lib/theme');
Expand All @@ -13,6 +14,7 @@ interface Props {
}

function TagList(props: Props) {
const collatorLocale = getCollatorLocale();
const style = useMemo(() => {
const theme = themeStyle(props.themeId);

Expand All @@ -29,13 +31,13 @@ function TagList(props: Props) {

const tags = useMemo(() => {
const output = props.items.slice();

const collator = getCollator(collatorLocale);
output.sort((a: any, b: any) => {
return a.title < b.title ? -1 : +1;
return collator.compare(a.title, b.title);
});

return output;
}, [props.items]);
}, [props.items, collatorLocale]);

const tagItems = useMemo(() => {
const output = [];
Expand Down
5 changes: 5 additions & 0 deletions packages/lib/BaseApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,11 @@ export default class BaseApplication {
refreshNotes = true;
}

if (action.type === 'SETTING_UPDATE_ONE' && action.key === 'locale') {
refreshNotes = true;
doRefreshFolders = 'now';
}

if (action.type === 'SMART_FILTER_SELECT') {
refreshNotes = true;
refreshNotesUseSelectedNoteId = true;
Expand Down
4 changes: 3 additions & 1 deletion packages/lib/components/shared/side-menu-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Folder from '../../models/Folder';
import BaseModel from '../../BaseModel';
import { FolderEntity, TagEntity } from '../../services/database/types';
import { getDisplayParentId, getTrashFolderId } from '../../services/trash';
import { getCollator } from '../../models/utils/getCollator';

interface Props {
folders: FolderEntity[];
Expand Down Expand Up @@ -72,6 +73,7 @@ export const renderFolders = (props: Props, renderItem: RenderFolderItem) => {

export const renderTags = (props: Props, renderItem: RenderTagItem) => {
const tags = props.tags.slice();
const collator = getCollator();
tags.sort((a, b) => {
// It seems title can sometimes be undefined (perhaps when syncing
// and before tag has been decrypted?). It would be best to find
Expand All @@ -83,7 +85,7 @@ export const renderTags = (props: Props, renderItem: RenderTagItem) => {
// Note: while newly created tags are normalized and lowercase
// imported tags might be any case, so we need to do case-insensitive
// sort.
return a.title.toLowerCase() < b.title.toLowerCase() ? -1 : +1;
return collator.compare(a.title, b.title);
});
const tagItems = [];
const order: string[] = [];
Expand Down
28 changes: 28 additions & 0 deletions packages/lib/models/Folder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,34 @@ describe('models/Folder', () => {
expect(sortedFolderTree[2].id).toBe(f6.id);
}));

it('should sort folders with special chars alphabetically', (async () => {
const unsortedFolderTitles = ['ç', 'd', 'c', 'Ä', 'b', 'a'].map(firstChar => `${firstChar} folder`);
for (const folderTitle of unsortedFolderTitles) {
await Folder.save({ title: folderTitle });
}

const folders = await Folder.allAsTree();
const sortedFolderTree = await Folder.sortFolderTree(folders);

// same set of titles, but in alphabetical order
const sortedFolderTitles = ['a', 'Ä', 'b', 'c', 'ç', 'd'].map(firstChar => `${firstChar} folder`);
expect(sortedFolderTree.map(f => f.title)).toEqual(sortedFolderTitles);
}));

it('should sort numbers ascending', (async () => {
const unsortedFolderTitles = ['10', '1', '2'].map(firstChar => `${firstChar} folder`);
for (const folderTitle of unsortedFolderTitles) {
await Folder.save({ title: folderTitle });
}

const folders = await Folder.allAsTree();
const sortedFolderTree = await Folder.sortFolderTree(folders);
laurent22 marked this conversation as resolved.
Show resolved Hide resolved

// same set of titles, but in ascending order
const sortedFolderTitles = ['1', '2', '10'].map(firstChar => `${firstChar} folder`);
expect(sortedFolderTree.map(f => f.title)).toEqual(sortedFolderTitles);
}));

it('should not allow setting a folder parent as itself', (async () => {
const f1 = await Folder.save({ title: 'folder1' });
const hasThrown = await checkThrowAsync(() => Folder.save({ id: f1.id, parent_id: f1.id }, { userSideValidation: true }));
Expand Down
15 changes: 14 additions & 1 deletion packages/lib/models/Folder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import syncDebugLog from '../services/synchronizer/syncDebugLog';
import ResourceService from '../services/ResourceService';
import { LoadOptions } from './utils/types';
import ActionLogger from '../utils/ActionLogger';

import { getTrashFolder } from '../services/trash';
import getConflictFolderId from './utils/getConflictFolderId';
import getTrashFolderId from '../services/trash/getTrashFolderId';
import { getCollator } from './utils/getCollator';
const { substrWithEllipsis } = require('../string-utils.js');

const logger = Logger.create('models/Folder');
Expand Down Expand Up @@ -298,8 +300,18 @@ export default class Folder extends BaseItem {
return output;
}

public static handleTitleNaturalSorting(items: FolderEntity[], options: any) {
if (options.order?.length > 0 && options.order[0].by === 'title') {
const collator = getCollator();
items.sort((a, b) => ((options.order[0].dir === 'ASC') ? 1 : -1) * collator.compare(a.title, b.title));
}
}

public static async all(options: FolderLoadOptions = null) {
let output: FolderEntity[] = await super.all(options);
if (options) {
this.handleTitleNaturalSorting(output, options);
}

if (options && options.includeDeleted === false) {
output = output.filter(f => !f.deleted_time);
Expand Down Expand Up @@ -768,9 +780,10 @@ export default class Folder extends BaseItem {
const output = folders ? folders : await this.allAsTree();

const sortFoldersAlphabetically = (folders: FolderEntityWithChildren[]) => {
const collator = getCollator();
folders.sort((a: FolderEntityWithChildren, b: FolderEntityWithChildren) => {
if (a.parent_id === b.parent_id) {
return a.title.localeCompare(b.title, undefined, { sensitivity: 'accent' });
return collator.compare(a.title, b.title);
}
return 0;
});
Expand Down
10 changes: 3 additions & 7 deletions packages/lib/models/Note.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { pull, removeElement, unique } from '../ArrayUtils';
import { LoadOptions, SaveOptions } from './utils/types';
import ActionLogger from '../utils/ActionLogger';
import { getDisplayParentId, getTrashFolderId } from '../services/trash';
import { getCollator } from './utils/getCollator';
const urlUtils = require('../urlUtils.js');
const { isImageMimeType } = require('../resourceUtils');
const { MarkupToHtml } = require('@joplin/renderer');
Expand Down Expand Up @@ -294,8 +295,7 @@ export default class Note extends BaseItem {

return noteFieldComp(a.id, b.id);
};

const collator = this.getNaturalSortingCollator();
const collator = getCollator();

return notes.sort((a: NoteEntity, b: NoteEntity) => {
if (noteOnTop(a) && !noteOnTop(b)) return -1;
Expand Down Expand Up @@ -1121,15 +1121,11 @@ export default class Note extends BaseItem {

public static handleTitleNaturalSorting(items: NoteEntity[], options: any) {
if (options.order.length > 0 && options.order[0].by === 'title') {
const collator = this.getNaturalSortingCollator();
const collator = getCollator();
items.sort((a, b) => ((options.order[0].dir === 'ASC') ? 1 : -1) * collator.compare(a.title, b.title));
}
}

public static getNaturalSortingCollator() {
return new Intl.Collator(undefined, { numeric: true, sensitivity: 'base' });
}

public static async createConflictNote(sourceNote: NoteEntity, changeSource: number): Promise<NoteEntity> {
const conflictNote = { ...sourceNote };
delete conflictNote.id;
Expand Down
12 changes: 12 additions & 0 deletions packages/lib/models/utils/getCollator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { currentLocale, languageCodeOnly } from '../../locale';

function getCollator(locale: string = getCollatorLocale()) {
return new Intl.Collator(locale, { numeric: true, sensitivity: 'accent' });
}

function getCollatorLocale() {
const collatorLocale = languageCodeOnly(currentLocale());
return collatorLocale;
}

export { getCollator, getCollatorLocale };
Loading