Resolve "Selection Highlight Oversteps Bounds in Web IDE"
What does this MR do?
This MR fixes WebIDE Monaco editor styling to remove several overlay issues.
TL;DR: Our styles were breaking Monaco. Monaco layers overlays (selection, diff and other highlights) over each line of code. The UI was broken because we set full opacity to diff highlight (instead of 0.2
) and we displayed text selection highlight with the highest z-index
when it was supposed to be the lowest.
Issue in detail
Each line in Monaco editor can have multiple overlays.
Example overlays:
The overlays have a certain order that's implemented in Monaco itself. For example, in the issue #216491 (closed) of selection overstepping boundary the overlays look as follows:
The issue of overlay overstepping boundary was caused by us forcing the .selected-text
z-index
. This broke the Monaco internal logic, which layers background tile over the .selected-text
highlight.
History: Setting full opacity to the diff highlight caused selected text highlight not being visible gitlab-foss#47771 (closed) and so we fixed it by setting z-index
on the selected text highlight
The solution
We must style the overlays the same way Monaco does to prevent this and future issues. In our case, it means:
- not setting
z-index
on text selection highlight so it can be overlayed with other layers - making the diff highlight colors transparent so they don't completely cover the overlays below them (this only affects light-mode, because we didn't change the values for dark mode)
Screenshots
Light mode
Dark mode
We didn't override the diff highlighting in dark mode and so this MR only fixes selection-related issues:
scenario | before | after |
---|---|---|
diff view (should look the same) | ||
selecting a function |
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Part of #216491 (closed)