From 25b96937f0f44a304b267ca519ba3f4632cc5380 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 10 Apr 2022 11:25:06 +0100 Subject: [PATCH] Revert "Revert "Desktop: Fixes #5686: Fixed Tags Order (#6136)"" This reverts commit 3725b14e047356e583fbf38d8b9f140bad6a4691. --- .../gui/MainScreen/commands/setTags.ts | 22 +++++++------------ packages/app-desktop/gui/TagList.tsx | 7 ++---- .../lib/components/shared/side-menu-shared.js | 16 ++------------ packages/lib/models/Tag.test.js | 22 ++++++++++++++++++- packages/lib/models/Tag.ts | 14 ++++++++++++ 5 files changed, 47 insertions(+), 34 deletions(-) diff --git a/packages/app-desktop/gui/MainScreen/commands/setTags.ts b/packages/app-desktop/gui/MainScreen/commands/setTags.ts index 494bf1311..edea5abca 100644 --- a/packages/app-desktop/gui/MainScreen/commands/setTags.ts +++ b/packages/app-desktop/gui/MainScreen/commands/setTags.ts @@ -1,6 +1,7 @@ import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService'; import { _ } from '@joplin/lib/locale'; import Tag from '@joplin/lib/models/Tag'; +import { TagEntity } from '@joplin/lib/services/database/types'; export const declaration: CommandDeclaration = { name: 'setTags', @@ -14,23 +15,16 @@ export const runtime = (comp: any): CommandRuntime => { noteIds = noteIds || context.state.selectedNoteIds; const tags = await Tag.commonTagsByNoteIds(noteIds); - const startTags = tags - .map((a: any) => { + const sortedTags = Tag.sortTags(tags); + const startTags = sortedTags + .map((a: TagEntity) => { return { value: a.id, label: a.title }; - }) - .sort((a: any, b: any) => { - // sensitivity accent will treat accented characters as differemt - // but treats caps as equal - return a.label.localeCompare(b.label, undefined, { sensitivity: 'accent' }); }); const allTags = await Tag.allWithNotes(); - const tagSuggestions = allTags.map((a: any) => { - return { value: a.id, label: a.title }; - }) - .sort((a: any, b: any) => { - // sensitivity accent will treat accented characters as differemt - // but treats caps as equal - return a.label.localeCompare(b.label, undefined, { sensitivity: 'accent' }); + const sortedAllTags = Tag.sortTags(allTags); + const tagSuggestions = sortedAllTags + .map((a: TagEntity) => { + return { value: a.id, label: a.title }; }); comp.setState({ diff --git a/packages/app-desktop/gui/TagList.tsx b/packages/app-desktop/gui/TagList.tsx index f4c0fb96e..e9276ca59 100644 --- a/packages/app-desktop/gui/TagList.tsx +++ b/packages/app-desktop/gui/TagList.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { useMemo } from 'react'; import { AppState } from '../app.reducer'; +import Tag from '@joplin/lib/models/Tag'; const { connect } = require('react-redux'); const { themeStyle } = require('@joplin/lib/theme'); @@ -28,11 +29,7 @@ function TagList(props: Props) { }, [props.style, props.themeId]); const tags = useMemo(() => { - const output = props.items.slice(); - - output.sort((a: any, b: any) => { - return a.title < b.title ? -1 : +1; - }); + const output = Tag.sortTags(props.items.slice()); return output; }, [props.items]); diff --git a/packages/lib/components/shared/side-menu-shared.js b/packages/lib/components/shared/side-menu-shared.js index 9e8a2548e..2cf358471 100644 --- a/packages/lib/components/shared/side-menu-shared.js +++ b/packages/lib/components/shared/side-menu-shared.js @@ -1,6 +1,7 @@ const Folder = require('../../models/Folder').default; const Setting = require('../../models/Setting').default; const BaseModel = require('../../BaseModel').default; +const Tag = require('../../models/Tag').default; const shared = {}; @@ -50,20 +51,7 @@ shared.renderFolders = function(props, renderItem) { }; shared.renderTags = function(props, renderItem) { - const tags = props.tags.slice(); - 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 - // the root cause but for now that will do. - // - // Fixes https://github.com/laurent22/joplin/issues/4051 - if (!a || !a.title || !b || !b.title) return 0; - - // 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; - }); + const tags = Tag.sortTags(props.tags.slice()); const tagItems = []; const order = []; for (let i = 0; i < tags.length; i++) { diff --git a/packages/lib/models/Tag.test.js b/packages/lib/models/Tag.test.js index 947714093..76cb30f6d 100644 --- a/packages/lib/models/Tag.test.js +++ b/packages/lib/models/Tag.test.js @@ -50,7 +50,7 @@ describe('models/Tag', function() { it('should return tags with note counts', (async () => { const folder1 = await Folder.save({ title: 'folder1' }); const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id }); - const note2 = await Note.save({ title: 'ma 2nd note', parent_id: folder1.id }); + const note2 = await Note.save({ title: 'ma 2nd note',parent_id: folder1.id }); const todo1 = await Note.save({ title: 'todo 1', parent_id: folder1.id, is_todo: 1, todo_completed: 1590085027710 }); await Tag.setNoteTagsByTitles(note1.id, ['un']); await Tag.setNoteTagsByTitles(note2.id, ['un']); @@ -156,4 +156,24 @@ describe('models/Tag', function() { expect(commonTagIds.includes(tagc.id)).toBe(true); })); + it('should sort tags', (async () => { + // test for tags with titles + const unsortedTags = [{ title: '@⏲15 min' },{ title: '#house' },{ title: '#coding' }, { title: '@⏲60 min' }, { title: '#wait' }, { title: '@⏲30 min' }]; + const sortedTags = Tag.sortTags(unsortedTags); + expect(sortedTags).toEqual([{ title: '@⏲15 min' }, { title: '@⏲30 min' }, { title: '@⏲60 min' }, { title: '#coding' }, { title: '#house' }, { title: '#wait' }]); + + // test for tags without titles + const unsortedTags2 = [{ id: '40' } , { id: '50' }, { id: '10' }, { id: '30' }, { id: '20' }]; + const sortedTags2 = Tag.sortTags(unsortedTags2); + expect(sortedTags2).toEqual([{ id: '40' } , { id: '50' }, { id: '10' }, { id: '30' }, { id: '20' }]); + + // test for tags with titles, without titles and empty tags + const unsortedTags3 = [{ id: '1' }, { id: '2', title: 'two' }, {}, { id: '3' }, { id: '4', title: 'four' }, { id: '5',title: 'five' }]; + const sortedTags3 = Tag.sortTags(unsortedTags3); + expect(sortedTags3).toEqual([{ id: '1' }, { id: '2', title: 'two' }, {}, { id: '3' }, { id: '5', title: 'five' }, { id: '4', title: 'four' }]); + + // test for empty list + const emptyListSort = Tag.sortTags([]); + expect(emptyListSort).toEqual([]); + })); }); diff --git a/packages/lib/models/Tag.ts b/packages/lib/models/Tag.ts index a25916dc1..4721e6f09 100644 --- a/packages/lib/models/Tag.ts +++ b/packages/lib/models/Tag.ts @@ -216,4 +216,18 @@ export default class Tag extends BaseItem { return tag; }); } + + static sortTags(tags: TagEntity[]) { + return tags.sort((a: TagEntity,b: TagEntity) => { + // It seems title can sometimes be undefined (perhaps when syncing and before tag has been decrypted?). It would be best to find the root cause but for now that will do. + // Fixes https://github.com/laurent22/joplin/issues/4051 + if (!a || !a.title || !b || !b.title) return 0; + + // 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.localeCompare(b.title, undefined, { sensitivity: 'accent' }); + }); + } + }