web: (RFC14) Add invalid keyword highlighting
Created by: ghost
Changes:
- Invalid search keywords are now decorated with a red wavy underline
- The search input has been refactored to use a
<div contentEditable>
- Enables rendering of styled text in the search input
Implements:
Notes:
- The invalid keyword tags are rendered using a HTML string. It's possible to use React elements with the attribute
suppressContentEditableWarning
, but that will require a refactor of the cursor position calculation, including how suggestions are added to the query, as that was coded for a single text<input>
. I'd like to leave this for a future PR so we can have time to discuss on the next feature to the search input and how that can be developed. The current additions are well isolated, and if it doesn't work out, the components can easily be switched back to using a single<input>
- Initial implementation informs of the invalid keyword using the
title
attribute. Future implementations may show quick-fix actions, but that will need some extra planning (previous item on refactoring) - I'm planning on a PR to split out the functions in
web/src/search/helpers.tsx
, the file is getting a bit long and might cause unnecessary conflicts on future code updates - I tried out Draft.js but couldn't find my way around getting it to render correctly with our styling, and get it to work correctly in a single line (also using plug-ins). But now with the contentEditable rendering and working correctly, maybe we can revisit Draft.js for 3.12
- While updating the regex I noticed that RegExr is using CodeMirror for its styled single line input. CodeMirror can also be an option for our use case
Updates:
- Dec/12/2019: Investigating usage of CodeMirror as a replacement to
ContentEditableInput
Merge request reports
Activity
Created by: rvantonder
Cool feature! Two things I noticed:
- The regex doesn't detect quoted search patterns, so we will highlight things like this:
I actually would not advocate for a regex approach to solve this, it is brittle (single and double quotes need to be considered, but it is dependent on the
patterntype
in effect, etc.).- The
patterntype
keyword is not recognized:
Ideally both these issues would be handled by RFC 75(the backend would recognize what is and isn't a keyword, and tell you which keywords are invalid, and their position). But that's still work in progress. I realize we may want to ship this highlight feature before a that workable solution exists, so for (1) I'm inclined to just recommend punting on trying to do this better with a regex pattern.
Created by: felixfbecker
One problem with
contenteditable
is that it allows rich content, so my default I could paste e.g. bold text into it, or even dangerous content containing scripts or event handlers. I just wanted to test this, but pasting actually doesn't work at all. Console output after pasting:Created by: attfarhan
I was able to copy-paste some bold text from the Notes app, but it results in a very odd query. Because content-editable allows rich content to be inputted, I imagine we may run into many issues like this:
We may have to sanitize the input somehow for this to make sense.
Also it looks like it allows multiple lines to be inserted in the search bar. Is that desirable?
Created by: codecov[bot]
Codecov Report
Merging #7026 into master will decrease coverage by
0.07%
. The diff coverage is34.16%
.@@ Coverage Diff @@ ## master #7026 +/- ## ========================================== - Coverage 39.23% 39.15% -0.08% ========================================== Files 1230 1225 -5 Lines 63533 62979 -554 Branches 6041 6137 +96 ========================================== - Hits 24924 24660 -264 + Misses 36326 36074 -252 + Partials 2283 2245 -38
Impacted Files Coverage Δ web/src/search/input/SearchPage.tsx 0% <ø> (ø)
web/src/search/input/SearchNavbarItem.tsx 0% <ø> (ø)
web/src/search/input/QueryInput.tsx 0% <0%> (ø)
web/src/search/input/ContentEditableInput.tsx 9.23% <9.23%> (ø)
web/src/search/helpers.tsx 80.13% <92.1%> (+4.02%)
web/src/enterprise/campaigns/list/CampaignNode.tsx 0% <0%> (-100%)
lsif/src/shared/models/util.ts 0% <0%> (-100%)
...rprise/campaigns/detail/changesets/presentation.ts 0% <0%> (-100%)
.../src/enterprise/campaigns/detail/BurndownChart.tsx 0% <0%> (-100%)
...rise/campaigns/detail/changesets/ChangesetNode.tsx 0% <0%> (-95%)
... and 114 more Created by: ghost
@attfarhan @felixfbecker You can now try to paste in rich text. If it does not render to plain text, please ping me so I can get the content you pasted, and debug it locally.
@rvantonder I've updated the regex to exclude keywords in quotes.
@lguychard The broken rendering seems to be an issue with chromium based browsers. I've updated the styling to an underline.