Skip to content

fix: adjust styling to avoid hiding IntelliSense popups#243

Merged
erickzhao merged 1 commit intomasterfrom
fix/intellisense-overflow-hidden
Jul 30, 2019
Merged

fix: adjust styling to avoid hiding IntelliSense popups#243
erickzhao merged 1 commit intomasterfrom
fix/intellisense-overflow-hidden

Conversation

@erickzhao
Copy link
Copy Markdown
Member

Problem

Currently, some styling leads to the Monaco editor's IntelliSense being hidden if it would overflow the window.

Screenshot:
image

Fix

I pinpointed the root cause of the problem to .mosaic-preview in our own src/less/mosaic-vendor.less file, which sets overflow: hidden.

Looking at the original vendor styling (link), it seems like they're not setting any overflow anywhere, so it seems to be on our end.

Since this vendor .less file was originally added in PR #14 to improve FPS, I wanted to test if removing the rule would cause a regression in the scrolling FPS:

  1. Screen capture with overflow: hidden
  2. Screen capture without

There doesn't seem to be any sort of meaningful regression here, and it seems that removing the rule solves the IntelliSense problem (see video 2).

@erickzhao erickzhao requested a review from felixrieseberg July 29, 2019 22:44
@erickzhao erickzhao changed the title fix: adjust styling to avoid hiding IntelliSense fix: adjust styling to avoid hiding IntelliSense popups Jul 29, 2019
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 97.105% when pulling 25268ae on fix/intellisense-overflow-hidden into f307964 on master.

Copy link
Copy Markdown
Member

@felixrieseberg felixrieseberg left a comment

Choose a reason for hiding this comment

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

🤔 I feel like there was probably a reason we added the hidden, but I can't remember. My fault for not leaving a comment somewhere, so let's get this in.

@erickzhao erickzhao merged commit 7035be3 into master Jul 30, 2019
@erickzhao erickzhao deleted the fix/intellisense-overflow-hidden branch July 30, 2019 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants