PonyMirrors/philomena
1
0
Fork 0
mirror of https://github.com/philomena-dev/philomena.git synced 2025-03-19 02:07:14 +01:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #462 from MareStare/fix/autocomplete-incorrect-match-highlight

Browse source 
Fix invalid highlighting of the prefix match for namespaced tags in autocomplete
This commit is contained in:
liamwhite 2025-03-18 08:53:31 -04:00 • committed by GitHub
commit fb963e80b2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 288 additions and 137 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
.typos.toml
View file

@ -5,6 +5,9 @@ extend-ignore-re = [
".*secret_key_base.*",
# Key constraints with encoded names
"fk_rails_[a-f0-9]+"
]
"fk_rails_[a-f0-9]+",
# Often used in tests as a partial match for `foo`
"fo",
"FO",
]

6
assets/js/autocomplete/history/index.ts
View file

@ -1,4 +1,4 @@
import { HistorySuggestion } from '../../utils/suggestions';
import { HistorySuggestionComponent } from '../../utils/suggestions-view';
import { InputHistory } from './history';
import { HistoryStore } from './store';
import { AutocompletableInput } from '../input';
@ -60,7 +60,7 @@ export function listen() {
* specified as an argument, it will return the maximum number of suggestions
* allowed by the input.
*/
export function listSuggestions(input: AutocompletableInput, limit?: number): HistorySuggestion[] {
export function listSuggestions(input: AutocompletableInput, limit?: number): HistorySuggestionComponent[] {
if (!input.hasHistory()) {
return [];
}
@ -70,5 +70,5 @@ export function listSuggestions(input: AutocompletableInput, limit?: number): Hi
return histories
.load(input.historyId)
.listSuggestions(value, limit ?? input.maxSuggestions)
.map(content => new HistorySuggestion(content, value.length));
.map(content => new HistorySuggestionComponent(content, value.length));
}

49
assets/js/autocomplete/index.ts
View file

@ -2,13 +2,14 @@ import { LocalAutocompleter } from '../utils/local-autocompleter';
import * as history from './history';
import { AutocompletableInput, TextInputElement } from './input';
import {
SuggestionsPopup,
SuggestionsPopupComponent,
Suggestions,
TagSuggestion,
TagSuggestionComponent,
Suggestion,
HistorySuggestion,
HistorySuggestionComponent,
ItemSelectedEvent,
} from '../utils/suggestions';
} from '../utils/suggestions-view';
import { prefixMatchParts } from '../utils/suggestions-model';
import { $$ } from '../utils/dom';
import { AutocompleteClient, GetTagSuggestionsRequest } from './client';
import { DebouncedCache } from '../utils/debounced-cache';
@ -33,7 +34,7 @@ function readHistoryConfig() {
class Autocomplete {
index: null | 'fetching' | 'unavailable' | LocalAutocompleter = null;
input: AutocompletableInput | null = null;
popup = new SuggestionsPopup();
popup = new SuggestionsPopupComponent();
client = new AutocompleteClient();
serverSideTagSuggestions = new DebouncedCache(this.client.getTagSuggestions.bind(this.client));
@ -120,7 +121,7 @@ class Autocomplete {
suggestions.tags = this.index
.matchPrefix(activeTerm, input.maxSuggestions - suggestions.history.length)
.map(result => new TagSuggestion({ ...result, matchLength: activeTerm.length }));
.map(suggestion => new TagSuggestionComponent(suggestion));
// Used for debugging server-side completions, to ensure local autocomplete
// doesn't prevent sever-side completions from being shown. Use these console
@ -146,7 +147,11 @@ class Autocomplete {
this.scheduleServerSideSuggestions(activeTerm, suggestions.history);
}
scheduleServerSideSuggestions(this: ActiveAutocomplete, term: string, historySuggestions: HistorySuggestion[]) {
scheduleServerSideSuggestions(
this: ActiveAutocomplete,
term: string,
historySuggestions: HistorySuggestionComponent[],
) {
const request: GetTagSuggestionsRequest = {
term,
@ -165,13 +170,25 @@ class Autocomplete {
// Truncate the suggestions to the leftover space shared with history suggestions.
const maxTags = this.input.maxSuggestions - historySuggestions.length;
const tags = response.suggestions.slice(0, maxTags).map(
suggestion =>
new TagSuggestion({
...suggestion,
matchLength: term.length,
}),
);
const tags = response.suggestions.slice(0, maxTags).map(suggestion => {
// Convert the server-side suggestions into the UI components.
// We are inferring the match parts assuming it's a prefix match here
// on frontend but best would be if backend would return the match parts
// directly.
if (suggestion.alias) {
return new TagSuggestionComponent({
alias: prefixMatchParts(suggestion.alias, term),
canonical: suggestion.canonical,
images: suggestion.images,
});
}
return new TagSuggestionComponent({
canonical: prefixMatchParts(suggestion.canonical, term),
images: suggestion.images,
});
});
this.showSuggestions({
history: historySuggestions,
@ -310,7 +327,7 @@ class Autocomplete {
this.input.element.dispatchEvent(newEvent);
if (ctrlKey || (suggestion instanceof HistorySuggestion && !shiftKey)) {
if (ctrlKey || (suggestion instanceof HistorySuggestionComponent && !shiftKey)) {
// We use `requestSubmit()` instead of `submit()` because it triggers the
// 'submit' event on the form. We have a handler subscribed to that event
// that records the input's value for history tracking.
@ -334,7 +351,7 @@ class Autocomplete {
const value = suggestion.value();
if (!activeTerm || suggestion instanceof HistorySuggestion) {
if (!activeTerm || suggestion instanceof HistorySuggestionComponent) {
element.value = value;
return;
}

28
assets/js/utils/__tests__/local-autocompleter.spec.ts
View file

@ -2,6 +2,7 @@ import { LocalAutocompleter } from '../local-autocompleter';
import { promises } from 'fs';
import { join } from 'path';
import { TextDecoder } from 'util';
import { MatchPart } from 'utils/suggestions-model';
describe('LocalAutocompleter', () => {
let mockData: ArrayBuffer;
@ -49,9 +50,16 @@ describe('LocalAutocompleter', () => {
function expectLocalAutocomplete(term: string, topK = 5) {
const localAutocomplete = new LocalAutocompleter(mockData);
const results = localAutocomplete.matchPrefix(term, topK);
const joinMatchParts = (parts: MatchPart[]) =>
parts.map(part => (typeof part === 'string' ? part : `{${part.matched}}`)).join('');
const actual = results.map(result => {
const canonical = `${result.canonical} (${result.images})`;
return result.alias ? `${result.alias} -> ${canonical}` : canonical;
if (result.alias) {
return `${joinMatchParts(result.alias)} -> ${result.canonical} (${result.images})`;
}
return `${joinMatchParts(result.canonical)} (${result.images})`;
});
return expect(actual);
@ -64,7 +72,7 @@ describe('LocalAutocompleter', () => {
it('should return suggestions for exact tag name match', () => {
expectLocalAutocomplete('safe').toMatchInlineSnapshot(`
[
"safe (6)",
"{safe} (6)",
]
`);
});
@ -72,7 +80,7 @@ describe('LocalAutocompleter', () => {
it('should return suggestion for an alias', () => {
expectLocalAutocomplete('flowers').toMatchInlineSnapshot(`
[
"flowers -> flower (1)",
"{flowers} -> flower (1)",
]
`);
});
@ -80,7 +88,7 @@ describe('LocalAutocompleter', () => {
it('should prefer canonical tag over an alias when both match', () => {
expectLocalAutocomplete('flo').toMatchInlineSnapshot(`
[
"flower (1)",
"{flo}wer (1)",
]
`);
});
@ -88,9 +96,9 @@ describe('LocalAutocompleter', () => {
it('should return suggestions sorted by image count', () => {
expectLocalAutocomplete(termStem).toMatchInlineSnapshot(`
[
"forest (3)",
"fog (1)",
"force field (1)",
"{fo}rest (3)",
"{fo}g (1)",
"{fo}rce field (1)",
]
`);
});
@ -98,7 +106,7 @@ describe('LocalAutocompleter', () => {
it('should return namespaced suggestions without including namespace', () => {
expectLocalAutocomplete('test').toMatchInlineSnapshot(`
[
"artist:test (1)",
"artist:{test} (1)",
]
`);
});
@ -106,7 +114,7 @@ describe('LocalAutocompleter', () => {
it('should return only the required number of suggestions', () => {
expectLocalAutocomplete(termStem, 1).toMatchInlineSnapshot(`
[
"forest (3)",
"{fo}rest (3)",
]
`);
});

63
assets/js/utils/__tests__/suggestion-model.spec.ts Normal file
View file

@ -0,0 +1,63 @@
import { prefixMatchParts } from '../suggestions-model.ts';
describe('prefixMatchParts', () => {
it('separates the prefix from the plain tag', () => {
expect(prefixMatchParts('foobar', 'foo')).toMatchInlineSnapshot(`
[
{
"matched": "foo",
},
"bar",
]
`);
});
it('separates the prefix from the namespaced tag', () => {
expect(prefixMatchParts('bruh:bar', 'bru')).toMatchInlineSnapshot(`
[
{
"matched": "bru",
},
"h:bar",
]
`);
});
it('separates the prefix after the namespace', () => {
expect(prefixMatchParts('foo:bazz', 'baz')).toMatchInlineSnapshot(`
[
"foo:",
{
"matched": "baz",
},
"z",
]
`);
});
it(`should ignore case when matching`, () => {
expect(prefixMatchParts('FOObar', 'foo')).toMatchInlineSnapshot(`
[
{
"matched": "FOO",
},
"bar",
]
`);
});
it(`should skip empty parts`, () => {
expect(prefixMatchParts('foo', 'foo')).toMatchInlineSnapshot(`
[
{
"matched": "foo",
},
]
`);
expect(prefixMatchParts('foo', 'bar')).toMatchInlineSnapshot(`
[
"foo",
]
`);
});
});

50
assets/js/utils/__tests__/suggestions.spec.ts → assets/js/utils/__tests__/suggestion-view.spec.ts
View file

@ -1,29 +1,31 @@
import {
SuggestionsPopup,
TagSuggestion,
TagSuggestionParams,
SuggestionsPopupComponent,
TagSuggestionComponent,
Suggestions,
HistorySuggestion,
HistorySuggestionComponent,
ItemSelectedEvent,
} from '../suggestions.ts';
} from '../suggestions-view.ts';
import { TagSuggestion } from 'utils/suggestions-model.ts';
import { afterEach } from 'vitest';
import { fireEvent } from '@testing-library/dom';
import { assertNotNull } from '../assert.ts';
const mockedSuggestions: Suggestions = {
history: ['foo bar', 'bar baz', 'baz qux'].map(content => new HistorySuggestion(content, 0)),
history: ['foo bar', 'bar baz', 'baz qux'].map(content => new HistorySuggestionComponent(content, 0)),
tags: [
{ images: 10, canonical: 'artist:assasinmonkey' },
{ images: 10, canonical: 'artist:hydrusbeta' },
{ images: 10, canonical: 'artist:the sexy assistant' },
{ images: 10, canonical: 'artist:devinian' },
{ images: 10, canonical: 'artist:moe' },
].map(tags => new TagSuggestion({ ...tags, matchLength: 0 })),
{ images: 10, canonical: ['artist:assasinmonkey'] },
{ images: 10, canonical: ['artist:hydrusbeta'] },
{ images: 10, canonical: ['artist:the sexy assistant'] },
{ images: 10, canonical: ['artist:devinian'] },
{ images: 10, canonical: ['artist:moe'] },
].map(suggestion => new TagSuggestionComponent(suggestion)),
};
function mockBaseSuggestionsPopup(includeMockedSuggestions: boolean = false): [SuggestionsPopup, HTMLInputElement] {
function mockBaseSuggestionsPopup(
includeMockedSuggestions: boolean = false,
): [SuggestionsPopupComponent, HTMLInputElement] {
const input = document.createElement('input');
const popup = new SuggestionsPopup();
const popup = new SuggestionsPopupComponent();
document.body.append(input);
popup.showForElement(input);
@ -38,7 +40,7 @@ function mockBaseSuggestionsPopup(includeMockedSuggestions: boolean = false): [S
const selectedItemClassName = 'autocomplete__item--selected';
describe('Suggestions', () => {
let popup: SuggestionsPopup | undefined;
let popup: SuggestionsPopupComponent | undefined;
let input: HTMLInputElement | undefined;
afterEach(() => {
@ -215,31 +217,31 @@ describe('Suggestions', () => {
it('should format suggested tags as tag name and the count', () => {
// The snapshots in this test contain a "narrow no-break space"
/* eslint-disable no-irregular-whitespace */
expectTagRender({ canonical: 'safe', images: 10 }).toMatchInlineSnapshot(`
expectTagRender({ canonical: ['safe'], images: 10 }).toMatchInlineSnapshot(`
{
"label": " safe 10",
"value": "safe",
}
`);
expectTagRender({ canonical: 'safe', images: 10_000 }).toMatchInlineSnapshot(`
expectTagRender({ canonical: ['safe'], images: 10_000 }).toMatchInlineSnapshot(`
{
"label": " safe 10 000",
"value": "safe",
}
`);
expectTagRender({ canonical: 'safe', images: 100_000 }).toMatchInlineSnapshot(`
expectTagRender({ canonical: ['safe'], images: 100_000 }).toMatchInlineSnapshot(`
{
"label": " safe 100 000",
"value": "safe",
}
`);
expectTagRender({ canonical: 'safe', images: 1000_000 }).toMatchInlineSnapshot(`
expectTagRender({ canonical: ['safe'], images: 1000_000 }).toMatchInlineSnapshot(`
{
"label": " safe 1 000 000",
"value": "safe",
}
`);
expectTagRender({ canonical: 'safe', images: 10_000_000 }).toMatchInlineSnapshot(`
expectTagRender({ canonical: ['safe'], images: 10_000_000 }).toMatchInlineSnapshot(`
{
"label": " safe 10 000 000",
"value": "safe",
@ -249,7 +251,7 @@ describe('Suggestions', () => {
});
it('should display alias -> canonical for aliased tags', () => {
expectTagRender({ images: 10, canonical: 'safe', alias: 'rating:safe' }).toMatchInlineSnapshot(
expectTagRender({ images: 10, canonical: 'safe', alias: ['rating:safe'] }).toMatchInlineSnapshot(
`
{
"label": " rating:safe → safe 10",
@ -262,7 +264,7 @@ describe('Suggestions', () => {
});
function expectHistoryRender(content: string) {
const suggestion = new HistorySuggestion(content, 0);
const suggestion = new HistorySuggestionComponent(content, 0);
const label = suggestion
.render()
.map(el => el.textContent)
@ -272,8 +274,8 @@ function expectHistoryRender(content: string) {
return expect({ label, value });
}
function expectTagRender(params: Omit<TagSuggestionParams, 'matchLength'>) {
const suggestion = new TagSuggestion({ ...params, matchLength: 0 });
function expectTagRender(params: TagSuggestion) {
const suggestion = new TagSuggestionComponent(params);
const label = suggestion
.render()
.map(el => el.textContent)

2
assets/js/utils/dom.ts
View file

@ -61,7 +61,7 @@ export function removeEl<E extends HTMLElement>(...elements: E[] | ConcatArray<E
export function makeEl<Tag extends keyof HTMLElementTagNameMap>(
tag: Tag,
attr?: Partial<HTMLElementTagNameMap[Tag]>,
children: HTMLElement[] = [],
children: (HTMLElement | string)[] = [],
): HTMLElementTagNameMap[Tag] {
const el = document.createElement(tag);
if (attr) {

42
assets/js/utils/local-autocompleter.ts
View file

@ -1,24 +1,7 @@
// Client-side tag completion.
import { UniqueHeap } from './unique-heap';
import store from './store';
export interface Result {
/**
* If present, then this suggestion is for a tag alias.
* If absent, then this suggestion is for the `canonical` tag name.
*/
alias?: null | string;
/**
* The canonical name of the tag (non-alias).
*/
canonical: string;
/**
* Number of images tagged with this tag.
*/
images: number;
}
import { prefixMatchParts, TagSuggestion } from './suggestions-model';
/**
* Opaque, unique pointer to tag data.
@ -186,7 +169,8 @@ export class LocalAutocompleter {
}
/**
* Perform a binary search to fetch all results matching a condition.
* Perform a binary search with a subsequent forward scan to fetch all results
* matching a `compare` condition.
*/
private scanResults(
getResult: (i: number) => TagReferenceIndex,
@ -234,7 +218,7 @@ export class LocalAutocompleter {
/**
* Find the top K results by image count which match the given string prefix.
*/
matchPrefix(prefixStr: string, k: number): Result[] {
matchPrefix(prefixStr: string, k: number): TagSuggestion[] {
if (prefixStr.length === 0) {
return [];
}
@ -268,16 +252,20 @@ export class LocalAutocompleter {
return results.topK(k).map((i: TagReferenceIndex) => {
const alias = this.decoder.decode(this.referenceToName(i, false));
const canonical = this.decoder.decode(this.referenceToName(i));
const result: Result = {
canonical,
images: this.getImageCount(i),
};
const images = this.getImageCount(i);
if (alias !== canonical) {
result.alias = alias;
if (alias === canonical) {
return {
canonical: prefixMatchParts(canonical, prefixStr),
images,
};
}
return result;
return {
alias: prefixMatchParts(alias, prefixStr),
canonical,
images,
};
});
}
}

78
assets/js/utils/suggestions-model.ts Normal file
View file

@ -0,0 +1,78 @@
/**
* Matched string is represented as an array of parts that matched and parts that didn't.
* It is designed to be this much generic to allow for matches in random places in case
* if we decide to support more complex matching.
*
* String parts that didn't match are represented as primitive strings. String parts
* that matched are represented as objects with a `matched` property.
*/
export type MatchPart = string | { matched: string };
interface CanonicalTagSuggestion {
/**
* No alias name is present for the canonical tag. It's declared here explicitly
* to make TypeScript pick up this field as the tagged union discriminator.
*/
alias?: undefined;
/**
* The canonical name of the tag (non-alias).
*/
canonical: MatchPart[];
/**
* Number of images tagged with this tag.
*/
images: number;
}
interface AliasTagSuggestion {
/**
* The alias name of the tag.
*/
alias: MatchPart[];
/**
* The canonical tag the alias points to.
*/
canonical: string;
/**
* Number of images tagged with this tag.
*/
images: number;
}
export type TagSuggestion = CanonicalTagSuggestion | AliasTagSuggestion;
/**
* Infers where the prefix match occurred in the target string according to
* the given prefix. It assumes that `target` either starts with `prefix` or
* contains the `prefix` after the namespace separator.
*/
export function prefixMatchParts(target: string, prefix: string): MatchPart[] {
return prefixMatchPartsImpl(target, prefix).filter(part => typeof part !== 'string' || part.length > 0);
}
function prefixMatchPartsImpl(target: string, prefix: string): MatchPart[] {
const targetLower = target.toLowerCase();
const prefixLower = prefix.toLowerCase();
if (targetLower.startsWith(prefixLower)) {
const matched = target.slice(0, prefix.length);
return [{ matched }, target.slice(matched.length)];
}
const separator = target.indexOf(':');
if (separator < 0) {
return [target];
}
const matchStart = separator + 1;
return [
target.slice(0, matchStart),
{ matched: target.slice(matchStart, matchStart + prefix.length) },
target.slice(matchStart + prefix.length),
];
}

100
assets/js/utils/suggestions.ts → assets/js/utils/suggestions-view.ts
View file

@ -1,69 +1,61 @@
import { makeEl } from './dom.ts';
import { MatchPart, TagSuggestion } from './suggestions-model.ts';
export interface TagSuggestionParams {
/**
* If present, then this suggestion is for a tag alias.
* If absent, then this suggestion is for the `canonical` tag name.
*/
alias?: null | string;
export class TagSuggestionComponent {
data: TagSuggestion;
/**
* The canonical name of the tag (non-alias).
*/
canonical: string;
/**
* Number of images tagged with this tag.
*/
images: number;
/**
* Length of the prefix in the suggestion that matches the prefix of the current input.
*/
matchLength: number;
}
export class TagSuggestion {
alias?: null | string;
canonical: string;
images: number;
matchLength: number;
constructor(params: TagSuggestionParams) {
this.alias = params.alias;
this.canonical = params.canonical;
this.images = params.images;
this.matchLength = params.matchLength;
constructor(data: TagSuggestion) {
this.data = data;
}
value(): string {
return this.canonical;
if (typeof this.data.canonical === 'string') {
return this.data.canonical;
}
return this.data.canonical.map(part => (typeof part === 'string' ? part : part.matched)).join('');
}
render(): HTMLElement[] {
const { alias: aliasName, canonical: canonicalName, images: imageCount } = this;
const label = aliasName ? `${aliasName} → ${canonicalName}` : canonicalName;
const { data } = this;
return [
makeEl('div', { className: 'autocomplete__item__content' }, [
makeEl('i', { className: 'fa-solid fa-tag' }),
makeEl('b', {
className: 'autocomplete__item__tag__match',
textContent: ` ${label.slice(0, this.matchLength)}`,
}),
makeEl('span', {
textContent: label.slice(this.matchLength),
}),
' ',
...this.renderLabel(),
]),
makeEl('span', {
className: 'autocomplete__item__tag__count',
textContent: ` ${TagSuggestion.formatImageCount(imageCount)}`,
textContent: ` ${TagSuggestionComponent.renderImageCount(data.images)}`,
}),
];
}
static formatImageCount(count: number): string {
renderLabel(): (HTMLElement | string)[] {
const { data } = this;
if (!data.alias) {
return TagSuggestionComponent.renderMatchParts(data.canonical);
}
return [...TagSuggestionComponent.renderMatchParts(data.alias), ` → ${data.canonical}`];
}
static renderMatchParts(parts: MatchPart[]): (HTMLElement | string)[] {
return parts.map(part => {
if (typeof part === 'string') {
return part;
}
return makeEl('b', {
className: 'autocomplete__item__tag__match',
textContent: part.matched,
});
});
}
static renderImageCount(count: number): string {
// We use the 'fr' (French) number formatting style with space-separated
// groups of 3 digits.
const formatter = new Intl.NumberFormat('fr', { useGrouping: true });
@ -72,7 +64,7 @@ export class TagSuggestion {
}
}
export class HistorySuggestion {
export class HistorySuggestionComponent {
/**
* Full query string that was previously searched and retrieved from the history.
*/
@ -111,11 +103,11 @@ export class HistorySuggestion {
}
}
export type Suggestion = TagSuggestion | HistorySuggestion;
export type Suggestion = TagSuggestionComponent | HistorySuggestionComponent;
export interface Suggestions {
history: HistorySuggestion[];
tags: TagSuggestion[];
history: HistorySuggestionComponent[];
tags: TagSuggestionComponent[];
}
export interface ItemSelectedEvent {
@ -132,7 +124,7 @@ interface SuggestionItem {
/**
* Responsible for rendering the suggestions dropdown.
*/
export class SuggestionsPopup {
export class SuggestionsPopupComponent {
/**
* Index of the currently selected suggestion. -1 means an imaginary item
* before the first item that represents the state where no item is selected.
@ -198,7 +190,7 @@ export class SuggestionsPopup {
}
}
setSuggestions(params: Suggestions): SuggestionsPopup {
setSuggestions(params: Suggestions): SuggestionsPopupComponent {
this.cursor = -1;
this.items = [];
this.container.innerHTML = '';
@ -219,7 +211,7 @@ export class SuggestionsPopup {
}
appendSuggestion(suggestion: Suggestion) {
const type = suggestion instanceof TagSuggestion ? 'tag' : 'history';
const type = suggestion instanceof TagSuggestionComponent ? 'tag' : 'history';
const element = makeEl(
'div',
@ -323,7 +315,7 @@ export class SuggestionsPopup {
* Returns the item's prototype that can be viewed as the item's type identifier.
*/
private itemType(index: number) {
return this.items[index].suggestion instanceof TagSuggestion ? 'tag' : 'history';
return this.items[index].suggestion instanceof TagSuggestionComponent ? 'tag' : 'history';
}
showForElement(targetElement: HTMLElement) {