Revert "Revert "Desktop: Fixes #5686: Fixed Tags Order (#6136)""

This reverts commit 3725b14e04.

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
+			const sortedTags = Tag.sortTags(tags);
-				.map((a: any) => {
+			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) => {
+			const sortedAllTags = Tag.sortTags(allTags);
+			const tagSuggestions = sortedAllTags
+				.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' });
 				});
 
 			comp.setState({

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();
+		const output = Tag.sortTags(props.items.slice());
-
-		output.sort((a: any, b: any) => {
-			return a.title < b.title ? -1 : +1;
-		});
 
 		return output;
 	}, [props.items]);

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();
+	const tags = Tag.sortTags(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 tagItems = [];
 	const order = [];
 	for (let i = 0; i < tags.length; i++) {

packages/lib/models/Tag.test.js

@@ -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([]);
+	}));
 });

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' });
+		});
+	}
+
 }