Skip to content
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(compass-editor, compass-query-bar): autocomplete icon COMPASS-8031 #6064

Merged
merged 5 commits into from
Jul 30, 2024

Conversation

VivianTNT
Copy link
Contributor

Description

COMPASS-8031

Added icons next to each autocompletion item based on which type of completion it is; clock for query history, rectangular box for base completions.

image

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@VivianTNT VivianTNT requested a review from Anemy July 26, 2024 18:52
@VivianTNT VivianTNT added feature flagged PRs labeled with this label will not be included in the release notes of the next release feat labels Jul 26, 2024
Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good! And the icons look nice. I'm thinking we can likely do more for the icons of our existing completions, left a comment on that.

@@ -21,6 +21,7 @@ export function mapMongoDBCompletionToCodemirrorCompletion(
? completion.value
: wrapField(completion.value, escape === 'always'),
detail: completion.meta?.startsWith('field') ? 'field' : completion.meta,
type: 'base-autocompleter',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this will start showing the rectangle box icon for all of our possible autocompletion types (field, query, bson, etc). This is shown in the screenshot. Should we instead show different icons for these different types? Maybe something similar to what VSCode does? https://code.visualstudio.com/docs/editor/intellisense#_types-of-completions

Alternatively, we could hide icons for now and do that as a separate pr. It likely won't be too much so I think I'd prefer if we do it here.

From a quick look I'm thinking right now we only need to add a new the regular box, which we can use for the functions, which would be BSON and query. Then we use the existing base-autocompleter/field rectangle box for the field autocompletions. Maybe there are more icons we can use for now? Not sure.
We get those non-field autocompletions from here: https://github.com/mongodb-js/devtools-shared/tree/main/packages/mongodb-constants
Maybe some of the json-schema completions are keywords.

},
'& .cm-completionLabel': {
flex: 1,
overflow: 'hidden',
textOverflow: 'ellipsis',
whiteSpace: 'nowrap',
lineHeight: '110%',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this used for?

Copy link
Contributor Author

@VivianTNT VivianTNT Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed each completion wasn't centered vertically and for some reason couldn't get it to work using other properties (display: flex interferes with the ellipses). lineHeight is supposed to be the actual height of one completion box but I didn't know how to get that value so the 110% was more or less guess and check. Can definitely look more into this!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, not a blocker or a big issue, was mostly curious. The css properties alignItems, alignSelf, justifyContent or justifySelf might be useful for the container or the classes here. They can help vertically align things in flex. It also may be that the icon is changing the height of the container. I can take more of a look later if you'd like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be because originally each of the completions were very slightly scrollable. I removed being able to scroll individual completions so that might have messed up the height, but not sure about this.

@VivianTNT VivianTNT merged commit d8f89bd into main Jul 30, 2024
28 checks passed
@VivianTNT VivianTNT deleted the COMPASS-8031-autocomplete-icon branch July 30, 2024 13:56
@VivianTNT VivianTNT restored the COMPASS-8031-autocomplete-icon branch July 30, 2024 15:43
@VivianTNT VivianTNT deleted the COMPASS-8031-autocomplete-icon branch July 30, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat feature flagged PRs labeled with this label will not be included in the release notes of the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants