-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat(tools): add search preview functionality #12470
Conversation
WalkthroughThis pull request introduces a new search functionality within the web application, implemented through a JavaScript file ( Changes
Tip OpenAI O1 model for chat
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
weblate/static/editor/tools/search.js (1)
18-122
: Consider breaking down the function into smaller functions for better readability and maintainability.The
searchPreview
function is quite large and could be broken down into smaller functions for better readability and maintainability. For example, you could extract the logic for fetching search results, filtering irrelevant results, and highlighting matching text into separate functions.Tools
GitHub Check: CodeQL
[failure] 70-70: Inefficient regular expression
This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '.'.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- weblate/static/editor/tools/search.js (1 hunks)
- weblate/static/styles/main.css (1 hunks)
- weblate/templates/base.html (1 hunks)
Additional context used
GitHub Check: CodeQL
weblate/static/editor/tools/search.js
[failure] 70-70: Inefficient regular expression
This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '.'.
Additional comments not posted (9)
weblate/static/editor/tools/search.js (4)
18-122
: The debounce implementation, client-side filtering, and highlighting of matching text are well implemented.
- The debounce implementation correctly prevents too many requests while the user is typing.
- The client-side filtering of irrelevant results is a good optimization to improve the search preview accuracy.
- The highlighting of matching text in the search results enhances the user experience.
Tools
GitHub Check: CodeQL
[failure] 70-70: Inefficient regular expression
This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '.'.
136-154
: LGTM!The
buildSearchQuery
function correctly builds the search query string in the expected format by concatenating the user input with the path lookup and filters (if provided).
164-184
: LGTM!The
highlightMatch
function correctly highlights the matching text in the search results using a regular expression.
170-170
: The regular expression is not likely to cause performance issues in this case.The static analysis tool CodeQL has flagged the regular expression at line 170-170 as potentially inefficient due to the use of the '.' character. However, this is not likely to cause performance issues in this case because:
- The search queries are typically short.
- The number of search results is limited by the
fetchMax
parameter in thesearchPreview
function.weblate/templates/base.html (1)
80-80
: LGTM!The script tag to load
search.js
is correctly added with the necessary attributes.weblate/static/styles/main.css (4)
2114-2126
: LGTM!The CSS styles for
#search-preview
are well-defined and enhance the visual presentation of the search results section. The changes are approved.
2128-2135
: Looks good!The CSS styles for
#search-preview > #search-result
are properly defined. The changes are approved.
2137-2139
: Hover styles look good!The hover styles for
#search-preview > #search-result
are properly defined to provide visual feedback. The changes are approved.
2141-2143
: Margin styles are good!The left margin styles for
#search-preview > #search-result > div
are properly defined. The changes are approved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The preview really makes sense for the filter field above, not this one. The filter supports all queries, while the search is a simple string to replace in the translation.
Oh!? I think the main operation here is concerned with the strings to replace. And filters are additional. In the current solution the preview is mainly based on the search string while considering the filters. I don't think adding search preview to filters is the best option. as users usually don't search for filters but for strings. |
This will make WeblateOrg#12470 easier to implement.
This will make WeblateOrg#12470 easier to implement.
This will make #12470 easier to implement.
dd73d08
to
01eab90
Compare
This commit adds the functionality to display a search preview when typing in the search input field. The search preview shows a list of search results that match the user's input. It also highlights the matching text in the search results. The search preview is displayed below the search input field and disappears after a delay when the input field loses focus. The commit includes the following changes: - Added a new file `weblate/static/editor/tools/search.js` that contains the JavaScript code for the search preview functionality. - Added CSS styles in `weblate/static/styles/main.css` to position and style the search preview element.
…function - Improved RegEx while Making the search result url relative. because the regular expression may cause exponential backtracking on strings containing many repetitions of '.' - Extracted the logic for rendering search results into a separate function called "showResults". This improves code readability and maintainability by separating concerns and reducing the complexity of the main function.
- Instead of displaying the total number of search results, it now shows the number of matching strings found. - Instead of getting project path from url get it from the Form data. - Update the `showResults` function to accept the `count` parameter, which represents the number of search results. - Modify the logic in the `showResults` function to use the `count` parameter instead of the length of the `results` array. - Adjust the CSS style for the `#results-num` element to increase the font size to 16px.
01eab90
to
eea451f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I've fixed merge conflicts and added a changelog entry.
if (count > 0) { | ||
const t = ngettext("%s matching string", "%s matching strings", count); | ||
$searchPreview.append( | ||
`<h4 id="results-num">${interpolate(t, [count])}</h4>`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a link to /search/?q=...
with the search used in the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! okay
Proposed changes
This commit adds the functionality to display a search preview when typing in the search input field. The search preview shows a list of search results that match the user's input. It also highlights the matching text in the search results. The search preview is displayed below the search input field and disappears after a delay when the input field loses focus.
The commit includes the following changes:
weblate/static/editor/tools/search.js
that contains the JavaScript code for the search preview functionality.weblate/static/styles/main.css
to position and style the search preview element.Related: #4106
Checklist
Other information
Note
GET api/units
with the queryq
is case insensitive. In contrast to the underlying search inSearch and Replace
. If the preview consumed it solely, its results are inaccurate. This is avoided with client side filtering for accurate preview considering case sensitivity.I think we should consider this. as case sensitive search is important for some parts of the application.
Preview
Summary by CodeRabbit
New Features
Style Enhancements
Documentation