fix: UI issues with code editor (#23332)

## Description
https://github.com/appsmithorg/appsmith/pull/22652 introduced new styles
to code in Appsmith. Along with it was some unintentional changes.
Bundle optimization
[PR](https://github.com/appsmithorg/appsmith/pull/21667) introduced,
CodeEditorFallback component which incorrectly renders borders and
causes UI to flicker. It also broke styles of lint message popover. This
PR fixes these issues.

Code changes
- Added 6px padding to code editor lines
- Use class variables instead of ref
- Removed border for JSEditor's CodeEditorFallback component
- Fixes lint message style
>
> Links to Notion, Figma or any other documents that might be relevant
to the PR
>
https://www.notion.so/appsmith/Visual-changes-to-code-in-Appsmith-8bb530c60ee844e5b44adec039bf9280#d99e8450f022439f845b5a0d7deb1d3f
>
#### PR fixes following issue(s)
Fixes # (issue number)
> if no issue exists, please create an issue and ask the maintainers
about this first
>
>
#### Media
> A video or a GIF is preferred. when using Loom, don’t embed because it
looks like it’s a GIF. instead, just link to the video
>
>
#### Type of change
- Bug fix (non-breaking change which fixes an issue)
>
>
>
## Testing
>
#### How Has This Been Tested?
- [x] Manual
>
>
#### Test Plan
Visual checks to test for changes as per
[this](https://www.notion.so/appsmith/Error-tooltip-for-data-fields-on-property-pane-Needs-design-810cdc489c3e4168a0b1b348730ac98c?utm_content=810cdc48-9c3e-4168-a0b1-b348730ac98c&utm_campaign=TGA9SH07N&n=slack&n=slack_link_unfurl&pvs=6)
Notion doc
>
#### Issues raised during DP testing
None
>
## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Test-plan-implementation#speedbreaker-features-to-consider-for-every-change)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans/_edit#areas-of-interest)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [x] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed

---------

Co-authored-by: Ravi Kumar Prasad <ravi@appsmith.com>
This commit is contained in:
arunvjn 2023-05-17 10:32:42 +05:30 committed by GitHub
parent 72bee7b26e
commit d1f1a19b66
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 113 additions and 83 deletions

View File

@ -89,9 +89,6 @@ const ResponseContainer = styled.div`
.react-tabs__tab-panel { .react-tabs__tab-panel {
overflow: hidden; overflow: hidden;
} }
.CodeMirror-code {
font-size: 12px;
}
`; `;
const ResponseMetaInfo = styled.div` const ResponseMetaInfo = styled.div`
display: flex; display: flex;
@ -332,7 +329,6 @@ export const responseTabComponent = (
input={{ input={{
value: isString(output) ? output : JSON.stringify(output, null, 2), value: isString(output) ? output : JSON.stringify(output, null, 2),
}} }}
isReadOnly
/> />
), ),
[API_RESPONSE_TYPE_OPTIONS.TABLE]: ( [API_RESPONSE_TYPE_OPTIONS.TABLE]: (
@ -347,7 +343,6 @@ export const responseTabComponent = (
value: isString(output) ? output : JSON.stringify(output, null, 2), value: isString(output) ? output : JSON.stringify(output, null, 2),
}} }}
isRawView isRawView
isReadOnly
/> />
), ),
}[responseType]; }[responseType];
@ -562,7 +557,6 @@ function ApiResponseView(props: Props) {
input={{ input={{
value: response?.body, value: response?.body,
}} }}
isReadOnly
/> />
) : responseTabs && ) : responseTabs &&
responseTabs.length > 0 && responseTabs.length > 0 &&
@ -627,7 +621,6 @@ function ApiResponseView(props: Props) {
? JSON.stringify(responseHeaders, null, 2) ? JSON.stringify(responseHeaders, null, 2)
: "", : "",
}} }}
isReadOnly
/> />
)} )}
</ResponseDataContainer> </ResponseDataContainer>

View File

@ -278,8 +278,8 @@ class CodeEditor extends Component<Props, State> {
annotations: Annotation[] = []; annotations: Annotation[] = [];
updateLintingCallback: UpdateLintingCallback | undefined; updateLintingCallback: UpdateLintingCallback | undefined;
private editorWrapperRef = React.createRef<HTMLDivElement>(); private editorWrapperRef = React.createRef<HTMLDivElement>();
currentLineNumber: number | null = null;
AIEnabled = false; AIEnabled = false;
lineRef: React.MutableRefObject<number | null> = React.createRef();
constructor(props: Props) { constructor(props: Props) {
super(props); super(props);
@ -902,15 +902,15 @@ class CodeEditor extends Component<Props, State> {
cm.setOption("matchBrackets", false); cm.setOption("matchBrackets", false);
} }
if (!this.props.borderLess) return; if (!this.props.borderLess) return;
if (this.lineRef.current !== null) { if (this.currentLineNumber !== null) {
cm.removeLineClass( cm.removeLineClass(
this.lineRef.current, this.currentLineNumber,
"background", "background",
"CodeMirror-activeline-background", "CodeMirror-activeline-background",
); );
} }
cm.addLineClass(line, "background", "CodeMirror-activeline-background"); cm.addLineClass(line, "background", "CodeMirror-activeline-background");
this.lineRef.current = line; this.currentLineNumber = line;
}; };
handleEditorFocus = (cm: CodeMirror.Editor) => { handleEditorFocus = (cm: CodeMirror.Editor) => {
@ -981,13 +981,13 @@ class CodeEditor extends Component<Props, State> {
line: cursor.line, line: cursor.line,
}, },
}); });
if (this.lineRef.current !== null) { if (this.currentLineNumber !== null) {
cm.removeLineClass( cm.removeLineClass(
this.lineRef.current, this.currentLineNumber,
"background", "background",
"CodeMirror-activeline-background", "CodeMirror-activeline-background",
); );
this.lineRef.current = null; this.currentLineNumber = null;
} }
if (this.props.onEditorBlur) { if (this.props.onEditorBlur) {
this.props.onEditorBlur(); this.props.onEditorBlur();

View File

@ -43,7 +43,7 @@ const editorBackground = (theme?: EditorTheme) => {
return bg; return bg;
}; };
const codeMirrorColors = { export const CodeEditorColors = {
KEYWORD: "#304eaa", KEYWORD: "#304eaa",
FOLD_MARKER: "#442334", FOLD_MARKER: "#442334",
STRING: "#1659df", STRING: "#1659df",
@ -128,7 +128,7 @@ export const EditorWrapper = styled.div<{
.cm-s-duotone-light.CodeMirror { .cm-s-duotone-light.CodeMirror {
border-radius: 0px; border-radius: 0px;
font-family: ${(props) => props.theme.fonts.code}; font-family: ${(props) => props.theme.fonts.code};
font-size: 13px; font-size: ${(props) => (props.isReadOnly ? "12px" : "13px")};
border: 1px solid border: 1px solid
${(props) => { ${(props) => {
switch (true) { switch (true) {
@ -149,14 +149,14 @@ export const EditorWrapper = styled.div<{
color: ${Colors.CHARCOAL}; color: ${Colors.CHARCOAL};
& { & {
span.cm-operator { span.cm-operator {
color: ${codeMirrorColors.OPERATOR}; color: ${CodeEditorColors.OPERATOR};
} }
} }
.cm-property { .cm-property {
color: hsl(21, 70%, 53%); color: hsl(21, 70%, 53%);
} }
.cm-keyword { .cm-keyword {
color: #304eaa; color: ${CodeEditorColors.KEYWORD};
} }
.CodeMirror-foldgutter { .CodeMirror-foldgutter {
@ -179,7 +179,7 @@ export const EditorWrapper = styled.div<{
} }
.cm-string, .cm-string,
.token.string { .token.string {
color: ${codeMirrorColors.STRING}; color: ${CodeEditorColors.STRING};
} }
/* json response in the debugger */ /* json response in the debugger */
@ -206,7 +206,7 @@ export const EditorWrapper = styled.div<{
.cm-def, .cm-def,
.cm-property + span + .cm-def, .cm-property + span + .cm-def,
.cm-def + span + .cm-def { .cm-def + span + .cm-def {
color: ${codeMirrorColors.FUNCTION_ARGS}; color: ${CodeEditorColors.FUNCTION_ARGS};
} }
.cm-atom + span + .cm-property, .cm-atom + span + .cm-property,
@ -225,7 +225,7 @@ export const EditorWrapper = styled.div<{
} }
span.cm-number { span.cm-number {
color: ${codeMirrorColors.NUMBER}; color: ${CodeEditorColors.NUMBER};
} }
.cm-s-duotone-light span.cm-variable-2, .cm-s-duotone-light span.cm-variable-2,
@ -368,8 +368,18 @@ export const EditorWrapper = styled.div<{
padding: ${(props) => props.theme.spaces[2]}px 0px; padding: ${(props) => props.theme.spaces[2]}px 0px;
background-color: ${(props) => props.disabled && "#eef2f5"}; background-color: ${(props) => props.disabled && "#eef2f5"};
cursor: ${(props) => (props.disabled ? "not-allowed" : "text")}; cursor: ${(props) => (props.disabled ? "not-allowed" : "text")};
pre.CodeMirror-line,
pre.CodeMirror-line-like {
padding: 0 ${(props) => props.theme.spaces[2]}px;
} }
} }
}
pre.CodeMirror-line,
pre.CodeMirror-line-like {
padding: 0 ${(props) => props.theme.spaces[3]}px;
}
${(props) => ${(props) =>
props.className === "js-editor" && props.className === "js-editor" &&
` `

View File

@ -9,13 +9,19 @@ import {
} from "./styles"; } from "./styles";
import { ContentKind } from "./types"; import { ContentKind } from "./types";
import type { EditorProps } from "components/editorComponents/CodeEditor"; import type { EditorProps } from "components/editorComponents/CodeEditor";
import { JS_OBJECT_START_STATEMENT } from "workers/Linting/constants";
export default function CodeEditorFallback({ export default function CodeEditorFallback({
input, input,
isReadOnly,
onInteracted, onInteracted,
placeholder, placeholder,
showLineNumbers,
showLoadingProgress, showLoadingProgress,
}: Pick<EditorProps, "input" | "placeholder"> & { }: Pick<
EditorProps,
"input" | "placeholder" | "showLineNumbers" | "isReadOnly"
> & {
onInteracted: () => void; onInteracted: () => void;
showLoadingProgress: boolean; showLoadingProgress: boolean;
}) { }) {
@ -26,7 +32,11 @@ export default function CodeEditorFallback({
if (!parsedValue) { if (!parsedValue) {
contentKind = ContentKind.PLACEHOLDER; contentKind = ContentKind.PLACEHOLDER;
fallbackToRender = placeholder || ""; fallbackToRender = placeholder || "";
} else if (typeof parsedValue === "string" && parsedValue.includes("{{")) { } else if (
typeof parsedValue === "string" &&
(parsedValue.includes("{{") ||
parsedValue.startsWith(JS_OBJECT_START_STATEMENT))
) {
contentKind = ContentKind.CODE; contentKind = ContentKind.CODE;
fallbackToRender = parsedValue; fallbackToRender = parsedValue;
} else if (Array.isArray(parsedValue) || typeof parsedValue === "object") { } else if (Array.isArray(parsedValue) || typeof parsedValue === "object") {
@ -48,7 +58,7 @@ export default function CodeEditorFallback({
} }
return ( return (
<ContentWrapper contentKind={contentKind}> <ContentWrapper contentKind={contentKind} showLineNumbers={showLineNumbers}>
{showLoadingProgress && ( {showLoadingProgress && (
<ProgressContainer> <ProgressContainer>
<SpinnerContainer> <SpinnerContainer>
@ -60,8 +70,10 @@ export default function CodeEditorFallback({
<HighlighedCodeContainer <HighlighedCodeContainer
className="LazyCodeEditor" className="LazyCodeEditor"
contentKind={contentKind} contentKind={contentKind}
isReadOnly={isReadOnly}
onFocus={onInteracted} onFocus={onInteracted}
onMouseEnter={onInteracted} onMouseEnter={onInteracted}
showLineNumbers={showLineNumbers}
tabIndex={0} tabIndex={0}
> >
<pre>{fallbackToRender}</pre> <pre>{fallbackToRender}</pre>

View File

@ -227,10 +227,12 @@ function LazyCodeEditor({ input, placeholder, ...otherProps }: EditorProps) {
<LazyEditorWrapper className="t--lazyCodeEditor-fallback"> <LazyEditorWrapper className="t--lazyCodeEditor-fallback">
<CodeEditorFallback <CodeEditorFallback
input={input} input={input}
isReadOnly={otherProps.isReadOnly}
onInteracted={() => { onInteracted={() => {
stateMachine.current.transition("PLACEHOLDER_INTERACTED"); stateMachine.current.transition("PLACEHOLDER_INTERACTED");
}} }}
placeholder={placeholder} placeholder={placeholder}
showLineNumbers={otherProps.showLineNumbers}
showLoadingProgress={showLoadingProgress} showLoadingProgress={showLoadingProgress}
/> />
</LazyEditorWrapper> </LazyEditorWrapper>

View File

@ -1,28 +1,36 @@
import styled, { keyframes } from "styled-components"; import styled, { keyframes } from "styled-components";
import { ContentKind } from "./types"; import { ContentKind } from "./types";
import { CodeEditorColors } from "../CodeEditor/styledComponents";
export const HighlighedCodeContainer = styled("div")<{ export const HighlighedCodeContainer = styled("div")<{
contentKind: ContentKind; contentKind: ContentKind;
showLineNumbers?: boolean;
isReadOnly?: boolean;
}>` }>`
width: 100%; width: 100%;
background-color: #fff !important; background-color: #fff !important;
font-family: monospace !important;
font-weight: 400 !important; font-weight: 400 !important;
line-height: 21px !important; line-height: 21px !important;
min-height: inherit; min-height: inherit;
padding-top: 6px !important; padding: 6px;
padding-left: 10px !important;
padding-right: 10px !important;
padding-bottom: 6px !important;
pre { pre {
font-family: ${(props) => props.theme.fonts.code};
margin: 0 !important; margin: 0 !important;
overflow: hidden !important; overflow: hidden !important;
font-size: 14px !important; font-size: ${(props) => (props.isReadOnly ? "12px" : "13px")} !important;
font-family: monospace !important;
padding: 0 !important; padding: 0 !important;
tab-size: 2 !important;
background: white !important; background: white !important;
${(props) => {
if (props.isReadOnly) {
return "padding-left: 35px !important";
}
if (props.showLineNumbers) {
return "padding-left: 47px !important";
}
}};
word-wrap: break-word !important; word-wrap: break-word !important;
white-space: pre-wrap !important; white-space: pre-wrap !important;
@ -30,7 +38,7 @@ export const HighlighedCodeContainer = styled("div")<{
color: ${({ contentKind }) => color: ${({ contentKind }) =>
contentKind === ContentKind.CODE contentKind === ContentKind.CODE
? "#063289" ? CodeEditorColors.KEYWORD
: contentKind === ContentKind.PLACEHOLDER : contentKind === ContentKind.PLACEHOLDER
? "#858282" ? "#858282"
: "inherit"} !important; : "inherit"} !important;
@ -43,6 +51,8 @@ export const LazyEditorWrapper = styled("div")`
export const ContentWrapper = styled("div")<{ export const ContentWrapper = styled("div")<{
contentKind: ContentKind; contentKind: ContentKind;
showLineNumbers?: boolean;
folding?: boolean;
}>` }>`
overflow: hidden; overflow: hidden;
width: 100%; width: 100%;
@ -51,6 +61,7 @@ export const ContentWrapper = styled("div")<{
min-height: 34px; min-height: 34px;
border: 1px solid; border: 1px solid;
border-color: inherit; border-color: inherit;
${(props) => props.showLineNumbers && "border: none"}
`; `;
const opacityAnimation = keyframes` const opacityAnimation = keyframes`

View File

@ -18,7 +18,6 @@ interface Props {
height: string; height: string;
folding: boolean; folding: boolean;
showLineNumbers?: boolean; showLineNumbers?: boolean;
isReadOnly?: boolean;
isRawView?: boolean; isRawView?: boolean;
containerHeight?: number; containerHeight?: number;
} }
@ -39,7 +38,7 @@ function ReadOnlyEditor(props: Props) {
: true, : true,
borderLess: true, borderLess: true,
folding: props.folding, folding: props.folding,
isReadOnly: props.isReadOnly, isReadOnly: true,
isRawView: props.isRawView, isRawView: props.isRawView,
containerHeight: props.containerHeight, containerHeight: props.containerHeight,
border: CodeEditorBorder.NONE, border: CodeEditorBorder.NONE,

View File

@ -237,55 +237,6 @@ export const CodemirrorHintStyles = createGlobalStyle<{
} }
} }
} }
.CodeMirror-Tern-hint-doc {
display: none;
&.visible {
display: block;
background-color: ${(props) =>
props.editorTheme === EditorTheme.DARK
? "#23292e"
: "#fff"} !important;
color: ${(props) =>
props.editorTheme === EditorTheme.DARK
? "#F4F4F4"
: "#1E242B"} !important;
max-height: 150px;
width: 250px;
font-size: 12px;
padding: 5px !important;
border: 1px solid !important;
border-color: ${(props) =>
props.editorTheme === EditorTheme.DARK
? "#23292e"
: "#DEDEDE"} !important;
box-shadow: 0px 4px 4px rgba(0, 0, 0, 0.12) !important;
overflow: scroll;
}
}
.CodeMirror-lint-tooltip {
border: none;
background: ${(props) =>
props.editorTheme === EditorTheme.DARK ? "#23292e" : "#fff"};
box-shadow: 0px 12px 28px -6px rgba(0, 0, 0, 0.32);
padding: 7px 12px;
border-radius: 0;
&.${LINT_TOOLTIP_JUSTIFIED_LEFT_CLASS}{
transform: translate(-100%);
}
}
.CodeMirror-lint-message {
margin-top: 5px;
margin-bottom: 5px;
font-family: ${(props) => props.theme.fonts.text};
color: #4B4848;
background-position: 0 2.8px;
padding-left: 20px;
}
.CodeMirror-lint-mark-warning{
background-image: url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAQAAAADCAYAAAC09K7GAAAAAXNSR0IArs4c6QAAAB1JREFUGFdjZICC/3sY/jO6MDAygvgwDpiGcWAqASvpC745SEL8AAAAAElFTkSuQmCC");
}
} }
.sql-hint-label{ .sql-hint-label{
color: #6D6D6D; color: #6D6D6D;
@ -306,4 +257,56 @@ export const CodemirrorHintStyles = createGlobalStyle<{
color: #fff color: #fff
} }
} }
.CodeMirror-Tern-hint-doc {
display: none;
&.visible {
display: block;
background-color: ${(props) =>
props.editorTheme === EditorTheme.DARK ? "#23292e" : "#fff"} !important;
color: ${(props) =>
props.editorTheme === EditorTheme.DARK
? "#F4F4F4"
: "#1E242B"} !important;
max-height: 150px;
width: 250px;
font-size: 12px;
padding: 5px !important;
border: 1px solid !important;
border-color: ${(props) =>
props.editorTheme === EditorTheme.DARK
? "#23292e"
: "#DEDEDE"} !important;
box-shadow: 0px 4px 4px rgba(0, 0, 0, 0.12) !important;
overflow: scroll;
}
}
.CodeMirror-lint-tooltip {
&& {
border: none;
background: ${(props) =>
props.editorTheme === EditorTheme.DARK ? "#23292e" : "#fff"};
box-shadow: 0px 12px 28px -6px rgba(0, 0, 0, 0.32);
padding: 7px 12px;
border-radius: 0;
&.${LINT_TOOLTIP_JUSTIFIED_LEFT_CLASS}{
transform: translate(-100%);
}
}
}
.CodeMirror-lint-message {
&& {
margin-top: 5px;
margin-bottom: 5px;
font-family: ${(props) => props.theme.fonts.text};
color: #4B4848;
background-position: 0 2.8px;
padding-left: 20px;
}
}
.CodeMirror-lint-mark-warning {
&& {
background-image: url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAQAAAADCAYAAAC09K7GAAAAAXNSR0IArs4c6QAAAB1JREFUGFdjZICC/3sY/jO6MDAygvgwDpiGcWAqASvpC745SEL8AAAAAElFTkSuQmCC");
}
}
`; `;