Skip to content
Snippets Groups Projects

web: (RFC14) Add invalid keyword highlighting

Closed Administrator requested to merge web/rfc14-keyword-highlighting into master

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

rfc14

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

Closed by avatar (Jul 6, 2025 2:12am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: rvantonder

    Cool feature! Two things I noticed:

    1. The regex doesn't detect quoted search patterns, so we will highlight things like this:

    Screen Shot 2019-12-04 at 12 58 11 AM

    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.).

    1. The patterntype keyword is not recognized:

    Screen Shot 2019-12-04 at 12 58 44 AM

    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: ghost

    @rvantonder thanks for the feedback! Indeed, RFC 75 looks like a better solution. For the patterntype keyword, I'll add it to the list of valid keywords. For the regex, I'll look into updating it to not match inside quotes.

  • Created by: lguychard

    On the homepage query field, the squiggles are truncated:

    image

    In the "main" query field, their display is a bit glitchy as you type, and they don't appear to extend to the full length of the invalid keyword:

    squiggles

    Could this be improved?

  • 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:

    image

  • Created by: attfarhan

    @felixfbecker I am able to paste normal text/bolded text. Are you trying to paste GIFs or something else?

  • Created by: felixfbecker

    I am pasting bold text copied from Microsoft Word

  • 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:

    2019-12-04 17 25 49

    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 is 34.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% <ø> (ø) :arrow_up:
    web/src/search/input/SearchNavbarItem.tsx 0% <ø> (ø) :arrow_up:
    web/src/search/input/QueryInput.tsx 0% <0%> (ø) :arrow_up:
    web/src/search/input/ContentEditableInput.tsx 9.23% <9.23%> (ø)
    web/src/search/helpers.tsx 80.13% <92.1%> (+4.02%) :arrow_up:
    web/src/enterprise/campaigns/list/CampaignNode.tsx 0% <0%> (-100%) :arrow_down:
    lsif/src/shared/models/util.ts 0% <0%> (-100%) :arrow_down:
    ...rprise/campaigns/detail/changesets/presentation.ts 0% <0%> (-100%) :arrow_down:
    .../src/enterprise/campaigns/detail/BurndownChart.tsx 0% <0%> (-100%) :arrow_down:
    ...rise/campaigns/detail/changesets/ChangesetNode.tsx 0% <0%> (-95%) :arrow_down:
    ... 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.

  • Created by: ghost

    @lguychard I'm ok with both the underline and dashed styling. For the component, I'm currently investigating how CodeMirror would suite this case. Monaco might also work if it has an non-hacky way to render as a single-line input.

  • Created by: beyang

    Dear all,

    This is your release captain speaking. :steam_locomotive::steam_locomotive::steam_locomotive:

    Branch cut for the 3.11 release is scheduled for tomorrow.

    Is this issue / PR going to make it in time? Please change the milestone accordingly. When in doubt, reach out!

    Thank you

Please register or sign in to reply
Loading