Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring effort to migrate the application's autocomplete functionality from an older jQuery-based library (autocomplete.js) to the more modern and flexible @algolia/autocomplete-js, specifically leveraging its headless core. This change aims to improve performance, maintainability, and user experience across various autocomplete instances, including attribute names, label values, and note search. The migration also addresses specific UI/UX issues and introduces new interaction patterns like Ctrl+Enter for confirmation. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request begins the migration from autocomplete.js to @algolia/autocomplete-js by refactoring the attribute name autocomplete to use a headless implementation with @algolia/autocomplete-core. The new implementation is well-structured, using requestAnimationFrame for positioning and a WeakMap for instance management. The PR also includes a backend fix to broaden attribute name suggestions and changes the submission shortcut in prompt dialogs to Ctrl+Enter.
My main feedback is regarding a memory and DOM leak in the new headless autocomplete implementation due to missing cleanup logic. Additionally, I've raised a concern about a hardcoded UX change in a generic prompt dialog, which could have unintended side effects in other parts of the application.
Note: Security Review did not run due to the size of the PR.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great step forward in migrating from the legacy jQuery-based autocomplete.js to the modern @algolia/autocomplete-js. The migration plan is thorough, and the implementation correctly uses the headless @algolia/autocomplete-core to handle the application's need for multiple autocomplete instances. The shift from jQuery events to explicit callbacks is a significant improvement in code clarity and maintainability. My review includes a few suggestions to enhance maintainability and user experience by addressing code duplication, function naming, and a potentially sluggish timeout.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request migrates the autocomplete functionality from autocomplete.js to @algolia/autocomplete-js, modernizing the codebase and using @algolia/autocomplete-core for multiple instances. However, it introduces a critical Cross-Site Scripting (XSS) vulnerability. The new implementation uses innerHTML to render search suggestions with user-controlled data without proper sanitization, potentially allowing attackers to execute arbitrary scripts. Sanitize all HTML content intended for highlighting using a library like DOMPurify before rendering it via innerHTML to mitigate this risk. Additionally, consider the suggestion regarding code clarity and maintainability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
这个 Pull Request 成功地将自动补全功能从旧的 autocomplete.js jQuery 插件迁移到了现代的 @algolia/autocomplete-js 库,特别是使用了其无头核心 @algolia/autocomplete-core 以实现更好的集成和控制。这些更改非常广泛,涉及属性自动补全、笔记搜索自动补全以及相关的 UI 组件和服务。新的实现结构良好,新的 autocomplete_core.ts 服务为无头设置提供了可复用的辅助函数。此次迁移还包括重构 jQuery 和 React 中的相关组件以适应新的 API,移除了旧的 polyfill,并更新了 E2E 测试使其更加健壮。总的来说,这是一次重要且执行良好的技术升级。我发现 PromptDialog 组件中有一个潜在的功能退化问题,需要处理一下。
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the client's autocomplete functionality from the legacy autocomplete.js to the modern, headless @algolia/autocomplete-core. This is a substantial architectural improvement, introducing a well-structured service layer (autocomplete_core.ts) and updating numerous components and services to use the new implementation. The changes are accompanied by new unit tests, and existing e2e tests have been adapted. My review found one minor issue where the state was not fully cleared when the autocomplete panel closes with an empty input, which could be a slight regression from the previous behavior.
|
@eliandoran This is quite a big migration, but all related behaviors are tested manually to ensure the migration doesn't break any existing functions. Please test a bit to see if I'm missing something. Would prefer an early merge to avoid repeatedly merging the main branch back. Thanks. |
There was a problem hiding this comment.
Manual testing seems to reveal some important issues:
- The hover style is completely missing.
- Typing too fast will slow down the application to a crawl. Reintroduce some kind of debouncing (there should already be a debouncing system but it seems that it's not being used).
- Double-check that the icon alignment and size is the same as previous.
- Sub-text (e.g. attributes) has a weird indentation.
Before implementing the fixes, it might be worth to look into:
- From what I see, a lot of things had to be implemented from scratch such as rendering the actual elements. In this case, what's the role of the Autocomplete Core? Can't we get rid of this dependency as well and handle our own auto-completion system?
This PR completes the migration from the legacy
autocomplete.jsintegration to headless@algolia/autocomplete-coreimplementations across the client.It replaces the old jQuery plugin usage in attribute and note autocomplete flows, updates React and non-React consumers to the new lifecycle, and preserves the existing product behavior around recent notes, command mode, keyboard shortcuts, IME handling, and custom selection events.
The change also introduces a shared headless autocomplete close registry, refreshes the CSS selectors for the new DOM structure, removes the obsolete
autocomplete.jsdependency and related jQuery polyfills, and cleans up leftover legacy selectors and dead styles.Test coverage was expanded with unit tests for attribute enter-key handling, global autocomplete closing, and the React
NoteAutocompletewrapper. Validation completed with client type-check/build success plus targeted and full server E2E runs; the full Playwright suite still reports pre-existing flaky tests, but the autocomplete-specific updated spec passes.All components listed below are tested manually to ensure they behave the same before and after migration.
Attribute Detail— verify attribute name autocomplete for both label and relation modes, including opening the panel, keyboard navigation, selecting an existing suggestion, and keeping the typed value when no suggestion is selected.Attribute Detail— verify label value autocomplete, especially creating a brand-new label value that has never appeared in suggestions before and saving it withCtrl+Enter/Cmd+Enter.Attribute Detail— verify relation target note autocomplete, including selecting an existing target note, creating a new target note when allowed, and saving after selection.Attribute Detail— verify outside click, tab switch, and dialog close behavior all close the autocomplete panel cleanly without leaving stale DOM behind.Promoted Attributes— verify label value autocomplete still opens, filters, selects, and preserves manually typed values.Promoted Attributes— verify relation target note selection through the ReactNoteAutocompletewrapper.Relation Map— verify relation name autocomplete still opens correctly, inserts the expected relation name, and does not regress keyboard behavior.Jump to Note Dialog— verify recent notes are shown on open, standard note search still works, selecting a note still jumps correctly, and the last query is restored when the dialog reopens.Jump to Note Dialog— verify command mode with>still lists commands, keyboard navigation works, and selecting a command triggers the expected action.Jump to Note Dialog— verifyEnter,Ctrl+Enter/Cmd+Enter, IME composition, and blur handling still match previous behavior.Empty Note Page— verify recent notes are shown on mount, selecting a note opens it correctly, and the contained autocomplete panel has no border.Add Link Dialog— verify recent notes on open, normal note search, keyboard selection, and link insertion after choosing a note.Move To Dialog— verify note selection still updates the destination and the move action uses the chosen note.Clone To Dialog— verify note selection still updates the destination and the clone action uses the chosen note.Include Note Dialog— verify note selection still updates the include target and submission still works.Note Type Chooser Dialog— verify note search and selection still set the target note type source correctly.Bulk Action: Move Note— verify note autocomplete still resolves the destination note and submission applies the move.Bulk Action: Add Relation— verify note autocomplete still resolves the target note and submission adds the relation.Bulk Action: Update Relation Target— verify note autocomplete still resolves the replacement target note and submission updates the relation.Collection Board— verify the board relation editor still searches and selects notes correctly.Collection Table Columns— verify table column note selection still works through the React wrapper.Render Widget— verify embedded note selection still updates the rendered target note correctly.Search Definition Options— verify both note autocomplete fields still sync text, selected note id, and submission state correctly.Global Headless Autocomplete Closing— verify opening or closing Bootstrap dialogs, changing tabs, and other global close triggers dismiss all open headless panels.Cross-platform Input Behavior— verify IME composition, arrow navigation,Escape, plainEnter, and modifier-assistedEnterflows on the main autocomplete surfaces above.close #5134