feat: adds prism editor and implements for sdkdocs #581

noahamac · 2021-04-13T05:48:24Z

Brought in the Prism editor by following the documentation very closely: https://www.npmjs.com/package/prism-react-renderer#usage and implementing for the SdkDocs component.

Also added integration with the search functionality.

https://looker.atlassian.net/browse/EMBED-1258
https://looker.atlassian.net/browse/EMBED-1154

Looking for advice on comments

Where to move this? Looking to get those components out and function to a util file. Should be accessible for RunIt as well. Would like to be able to bring into Data Dict and Lookml Diagram at some point too.
Search integration feedback. This is obviously unsophisticated, should we fuzzy match? Make case insensitive? Style a smaller token? Is it annoying that the main page zoomstotop? Should I add a prop to disable when searching?
I'm having a hard time finding ApiExplorer's node_modules for where to import this. And TS doesn't seem to like the global addition... Any thoughts?

Follow up PR could replace Ace usage for RunIt response.

noahamac · 2021-04-13T05:49:00Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+  display: table-cell;
+`
+
+export const PrismEditor: FC<PrismEditorProps> = ({


Where to move this? Looking to get those components out and function to a util file. Should be accessible for RunIt as well. Would like to be able to bring into Data Dict and Lookml Diagram at some point too. What's the overhead on making a new /packages/ ?

Remember that RunIt can be used on its own so this should live in RunIt if it's needed there as we cannot have any @looker/api-explorer dependencies in the @looker/run-it package.

And what if it needs to be used by both ApiExplorer and RunIt? (And DD and Lookml Diagram)? Are there are docs on creating a new packages item? Would this be a good candidate for that?

If it needs to be used by both ApiExplorer and RunIt, like in this case, then it should live in RunIt. I don't believe we have any documentation on how to create a new package but I'd be happy to work on that with you

noahamac · 2021-04-13T05:49:19Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+                {line.map((token, key) => {
+                  const tokenProps = getTokenProps({ token, key })
+                  const text = tokenProps.children
+                  if (pattern !== '' && text.includes(pattern)) {


Search integration feedback. This is obviously unsophisticated, should we fuzzy match? Make case insensitive? Style a smaller token? Is it annoying that the main page zoomstotop? Should I add a prop to disable when searching?

I don't think it's bad as is. There are times where it highlights the whole line and is a bit confusing but it's not a big deal. I do think we should open an issue to revisit highlighting across the whole app though because some of it got broken in the latest styling round

noahamac · 2021-04-13T05:49:52Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+// import { Prism } from "prism-react-renderer"
+import dracula from 'prism-react-renderer/themes/dracula'
+
+// (typeof global !== "undefined" ? global : window).Prism = Prism


I'm having a hard time finding ApiExplorer's node_modules for where to import this. And TS doesn't seem to like the global addition... Any thoughts?

For some reason we need to do this for kotlin, c#, swift etc support https://www.npmjs.com/package/prism-react-renderer#faq

I am not very familiar with how all things works under the hood. Maybe @bryans99 could chime in on this. Could we somehow lazy load the languages based on the user's sdk language preference?

noahamac · 2021-04-13T05:53:49Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+        <Pre className={className} style={style}>
+          {tokens.map((line, i) => (
+            <Line key={i} {...getLineProps({ line, key: i })}>
+              <LineNo>{i + 1}</LineNo>


Do we like the line number? Can ditch this

I don't have a preference

Line number can be useful for people discussing the code. Let's leave it in until we have people asking to remove it. Or we add a toggle/config setting when we start supporting APIX config settings more fully.

noahamac · 2021-04-13T05:56:59Z

new editor img: https://drive.google.com/file/d/1hy4GIEgvI8ZFJnKXXkhQcdN7ZleC_QNd/view?usp=sharing&resourcekey=0-kjhrx1HEFnuoTe8J-nNeQg

new editor search: https://drive.google.com/file/d/1bnjVbza-Fhn5TW4nokj_peZmtQ2r9TLH/view?usp=sharing&resourcekey=0-JGxc-1CAXRTUOIlJVp41lw

josephaxisa

Nice progress! A few thoughts/comments below

josephaxisa · 2021-04-13T14:20:19Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+  display: table-cell;
+`
+
+export const PrismEditor: FC<PrismEditorProps> = ({


Remember that RunIt can be used on its own so this should live in RunIt if it's needed there as we cannot have any @looker/api-explorer dependencies in the @looker/run-it package.

josephaxisa · 2021-04-13T15:19:30Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+  const getPrismLanguage = (language: string) => {
+    if (language === 'golang') {
+      return 'go'
+    }
+    return language as Language
+  }


This is ok for now, but we can change the generator language/label to Go in @looker/sdk-codegen. It was changed to Golang to avoid exactly this for Ace :)

josephaxisa · 2021-04-13T15:22:00Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+        <Pre className={className} style={style}>
+          {tokens.map((line, i) => (
+            <Line key={i} {...getLineProps({ line, key: i })}>
+              <LineNo>{i + 1}</LineNo>


I don't have a preference

josephaxisa · 2021-04-13T15:24:39Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+                {line.map((token, key) => {
+                  const tokenProps = getTokenProps({ token, key })
+                  const text = tokenProps.children
+                  if (pattern !== '' && text.includes(pattern)) {


I don't think it's bad as is. There are times where it highlights the whole line and is a bit confusing but it's not a big deal. I do think we should open an issue to revisit highlighting across the whole app though because some of it got broken in the latest styling round

josephaxisa · 2021-04-13T15:26:30Z

packages/api-explorer/src/components/DocSDKs/DocSDKs.tsx

@@ -77,7 +77,7 @@ export const DocSDKs: FC<LanguageSDKProps> = ({ api, method, type }) => {
                : gen.declareType('', item as IType)
              return (
                <TabPanel key={language}>
-                  <DocCode language={language} code={code} />
+                  <DocCode language={language} code={code} editor="prism" />


Perhaps the prop should say editor="ace" for places where we are still using ace so that it's easier to find and replace later on. This is completely fine as is though, just throwing a suggestion out there.

josephaxisa · 2021-04-13T15:28:55Z

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

+// import { Prism } from "prism-react-renderer"
+import dracula from 'prism-react-renderer/themes/dracula'
+
+// (typeof global !== "undefined" ? global : window).Prism = Prism


I am not very familiar with how all things works under the hood. Maybe @bryans99 could chime in on this. Could we somehow lazy load the languages based on the user's sdk language preference?

jkaster

Minor feedback

jkaster · 2021-04-13T18:45:58Z

packages/api-explorer/src/components/DocCode/DocCode.tsx

 }

 export const DocCode: FC<DocCodeProps> = ({
  code,
  language = 'json',
  fontSize = 16,
  width = 'auto',
+  editor = 'ace',


Default to prism?

packages/api-explorer/src/components/DocCode/PrismEditor.tsx

Co-authored-by: John Kaster <kaster@google.com>

jkaster

This is a good step in the right direction

adds prism editor and implements for sdkdocs

0fd6e04

google-cla bot added the cla: yes label Apr 13, 2021

noahamac changed the title ~~adds prism editor and implements for sdkdocs~~ feat: adds prism editor and implements for sdkdocs Apr 13, 2021

noahamac commented Apr 13, 2021

View reviewed changes

noahamac requested review from josephaxisa and jkaster and removed request for jkaster April 13, 2021 05:50

noahamac commented Apr 13, 2021

View reviewed changes

josephaxisa reviewed Apr 13, 2021

View reviewed changes

jkaster reviewed Apr 13, 2021

View reviewed changes

noahamac and others added 4 commits April 13, 2021 11:49

Update packages/api-explorer/src/components/DocCode/PrismEditor.tsx

612f90d

Co-authored-by: John Kaster <kaster@google.com>

Merge branch 'main' into noahamac/introduces-prism-editor

3c65a26

adds prism editor default to apix

b2b00ab

adds clike styling for unstyled

d4741f5

jkaster approved these changes Apr 13, 2021

View reviewed changes

jkaster merged commit aea0424 into main Apr 13, 2021

jkaster deleted the noahamac/introduces-prism-editor branch April 13, 2021 19:34

noahamac mentioned this pull request Apr 14, 2021

feat: introduces prism-editor package; implements read-only usage #588

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds prism editor and implements for sdkdocs #581

feat: adds prism editor and implements for sdkdocs #581

noahamac commented Apr 13, 2021

noahamac Apr 13, 2021 •

edited

Loading

josephaxisa Apr 13, 2021

noahamac Apr 13, 2021

josephaxisa Apr 13, 2021

noahamac Apr 13, 2021 •

edited

Loading

josephaxisa Apr 13, 2021

noahamac Apr 13, 2021

noahamac Apr 13, 2021

josephaxisa Apr 13, 2021

noahamac Apr 13, 2021

josephaxisa Apr 13, 2021

jkaster Apr 13, 2021

noahamac commented Apr 13, 2021

josephaxisa left a comment

josephaxisa Apr 13, 2021

josephaxisa Apr 13, 2021

josephaxisa Apr 13, 2021

josephaxisa Apr 13, 2021

josephaxisa Apr 13, 2021

josephaxisa Apr 13, 2021

jkaster left a comment

jkaster Apr 13, 2021

jkaster left a comment

feat: adds prism editor and implements for sdkdocs #581

feat: adds prism editor and implements for sdkdocs #581

Conversation

noahamac commented Apr 13, 2021

noahamac Apr 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noahamac Apr 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noahamac commented Apr 13, 2021

josephaxisa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkaster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkaster left a comment

Choose a reason for hiding this comment

noahamac Apr 13, 2021 •

edited

Loading

noahamac Apr 13, 2021 •

edited

Loading