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

playground: Add left panel and use brand colors #5838

Merged
merged 3 commits into from
Jul 19, 2023
Merged

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jul 17, 2023

Summary

This PR uses Astral's brand color in the playground more consistently and adds a left panel to switch between file and settings.

Test Plan

Screencast.from.2023-07-17.19-35-11.webm

I promise it shows an error when the settings file contains a syntax errors. It just happens that it wasn't part of the selection that I recorded.

@MichaReiser
Copy link
Member Author

MichaReiser commented Jul 17, 2023

@MichaReiser MichaReiser added the playground A playground-specific issue label Jul 17, 2023
@MichaReiser MichaReiser marked this pull request as ready for review July 17, 2023 17:38
@MichaReiser
Copy link
Member Author

@charliermarsh feel free to make changes if you don't like specific details. This is just not-designer Micha who tried to follow our brand guideline

@github-actions
Copy link
Contributor

github-actions bot commented Jul 17, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.8±0.11ms     3.8 MB/sec    1.07     11.6±0.10ms     3.5 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.2±0.02ms     7.7 MB/sec    1.05      2.3±0.03ms     7.3 MB/sec
formatter/numpy/globals.py                 1.00    243.8±3.66µs    12.1 MB/sec    1.04    253.7±2.67µs    11.6 MB/sec
formatter/pydantic/types.py                1.00      4.7±0.05ms     5.4 MB/sec    1.07      5.0±0.09ms     5.1 MB/sec
linter/all-rules/large/dataset.py          1.00     15.2±0.18ms     2.7 MB/sec    1.00     15.2±0.16ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.02      3.9±0.04ms     4.3 MB/sec    1.00      3.8±0.03ms     4.3 MB/sec
linter/all-rules/numpy/globals.py          1.01    509.4±4.87µs     5.8 MB/sec    1.00    503.6±7.00µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.9±0.08ms     3.7 MB/sec    1.00      6.8±0.08ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.00      7.7±0.08ms     5.3 MB/sec    1.00      7.8±0.07ms     5.2 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1656.6±16.60µs    10.1 MB/sec    1.00  1645.4±15.29µs    10.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    184.9±1.15µs    16.0 MB/sec    1.00    184.5±6.97µs    16.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.5±0.03ms     7.3 MB/sec    1.00      3.5±0.05ms     7.3 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.1±0.14ms     3.7 MB/sec    1.05     11.6±0.13ms     3.5 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.1±0.03ms     7.8 MB/sec    1.04      2.2±0.03ms     7.4 MB/sec
formatter/numpy/globals.py                 1.00    244.5±4.79µs    12.1 MB/sec    1.04    254.1±6.85µs    11.6 MB/sec
formatter/pydantic/types.py                1.00      4.7±0.05ms     5.4 MB/sec    1.05      4.9±0.07ms     5.2 MB/sec
linter/all-rules/large/dataset.py          1.02     15.6±0.26ms     2.6 MB/sec    1.00     15.2±0.14ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.1±0.05ms     4.1 MB/sec    1.00      4.0±0.05ms     4.1 MB/sec
linter/all-rules/numpy/globals.py          1.00    488.5±6.88µs     6.0 MB/sec    1.00    488.2±8.04µs     6.0 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.0±0.09ms     3.6 MB/sec    1.00      7.0±0.08ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.0±0.09ms     5.1 MB/sec    1.00      7.9±0.06ms     5.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1669.2±19.65µs    10.0 MB/sec    1.00  1660.7±19.43µs    10.0 MB/sec
linter/default-rules/numpy/globals.py      1.00    190.7±3.01µs    15.5 MB/sec    1.00    191.0±3.58µs    15.5 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.5±0.05ms     7.2 MB/sec    1.00      3.5±0.04ms     7.2 MB/sec

@MichaReiser MichaReiser force-pushed the merge-editor-states branch from d1c2923 to 8e78005 Compare July 18, 2023 06:01
Base automatically changed from merge-editor-states to main July 18, 2023 06:08
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

LGTM! I might do a follow-up pass when I have time on some of the brand-specific stuff like the button styling, but this is great.

)}
{...otherProps}
>
{children}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should use small caps for this, like the button on the top right of the homepage: https://astral.sh/. Maybe the same font too? Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied over some styles from astral. Is it okay to commit the fonts?

<div className="hidden sm:block">
<ShareButton key={edit} onShare={onShare} />
</div>
<div className="hidden sm:block mx-6 lg:mx-4 w-px h-6 bg-gray-200 dark:bg-gray-700" />
<Divider />
Copy link
Member

Choose a reason for hiding this comment

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

Are the dividers different colors? I assume it's just an artifact of the recording or something.

Screen Shot 2023-07-18 at 8 49 18 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so... They all render the same HTML
image

type SideBarProps = {
selected: Tool;
onSelectTool(tool: Tool): void;
};
Copy link
Member

Choose a reason for hiding this comment

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

I always just inline single-use definitions like this, e.g.:

export default function SideBar({ selected, onSelectTool }: { selected: Tool, ... } { ... }

But it's entirely subjective :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't do that. 😛

I find it extremely hard to read, always mess up the syntax when writing the types, and you need to refactor them out once you start adding new props.

Copy link
Member

Choose a reason for hiding this comment

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

...you need to refactor them out once you start adding new props.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Lol, don't get me started. I'm more opinionated on that than using nightly :P

Copy link
Member

Choose a reason for hiding this comment

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

Hahahaha. Okay I'm moving on. But I love "Charlie's style" for prop types.

@charliermarsh
Copy link
Member

I'm triggered by the juxtaposition of the dark purple (header and sidebar) against the black (editor) but I can take on this burden:

Screen Shot 2023-07-18 at 8 50 56 PM

@MichaReiser
Copy link
Member Author

MichaReiser commented Jul 19, 2023

juxtaposition

I'm not sure if I made it worse 😅

My idea was to use space for the header and side bar. It looks kind of good for the side bar, but we're not supposed to use our logo on space. I now use galaxy for the code and header, but space for the sidebar. Let me know what you think

Black code
image

Galaxy code

This would match the light theme where the header and code area have the same background.

image

Subject: [PATCH] Add right panel
---
Index: playground/src/Editor/MonacoThemes.tsx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/playground/src/Editor/MonacoThemes.tsx b/playground/src/Editor/MonacoThemes.tsx
--- a/playground/src/Editor/MonacoThemes.tsx	(revision 6e1b1c0f364b369f0aebe511a0d70a87e5199f66)
+++ b/playground/src/Editor/MonacoThemes.tsx	(date 1689751756944)
@@ -389,7 +389,7 @@
         "tab.inactiveForeground": "#565b66",
         "tab.unfocusedActiveForeground": "#565b66",
         "tab.unfocusedInactiveForeground": "#565b66",
-        "editor.background": "#0b0e14",
+        "editor.background": "#261230",
         "editor.foreground": "#bfbdb6",
         "editorLineNumber.foreground": "#6c738099",
         "editorLineNumber.activeForeground": "#6c7380e6",
@@ -408,7 +408,7 @@
         "editor.findMatchHighlightBorder": "#5f4c7266",
         "editor.findRangeHighlightBackground": "#6c598040",
         "editor.rangeHighlightBackground": "#6c598033",
-        "editor.lineHighlightBackground": "#131721",
+        "editor.lineHighlightBackground": "#30173D",
         "editorLink.activeForeground": "#e6b450",
         "editorWhitespace.foreground": "#6c738099",
         "editorIndentGuide.background": "#6c738033",
Index: playground/src/Editor/SideBar.tsx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/playground/src/Editor/SideBar.tsx b/playground/src/Editor/SideBar.tsx
--- a/playground/src/Editor/SideBar.tsx	(revision 6e1b1c0f364b369f0aebe511a0d70a87e5199f66)
+++ b/playground/src/Editor/SideBar.tsx	(date 1689751464419)
@@ -10,7 +10,7 @@
 
 export default function SideBar({ selected, onSelectTool }: SideBarProps) {
   return (
-    <ul className="w-12 flex-initial border-r bg-galaxy flex flex-col items-stretch">
+    <ul className="w-12 flex-initial border-r bg-space flex flex-col items-stretch">
       <SideBarEntry
         title="Source"
         onClick={() => onSelectTool("Source")}

@MichaReiser MichaReiser force-pushed the left-panel branch 2 times, most recently from 6e1b1c0 to fd4d6b7 Compare July 19, 2023 07:34
@charliermarsh
Copy link
Member

Let's run with what you had initially, I need to think on it a bit more and maybe play around with it but I def don't want to block shipping.

@MichaReiser MichaReiser merged commit 9ed7cee into main Jul 19, 2023
@MichaReiser MichaReiser deleted the left-panel branch July 19, 2023 14:33
@MichaReiser
Copy link
Member Author

MichaReiser commented Jul 19, 2023

Let's run with what you had initially, I need to think on it a bit more and maybe play around with it but I def don't want to block shipping.

Feel free to delegate the changes once you know how you want them to be. I enjoyed this a lot (feel free to do it yourself, if you find it fun too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
playground A playground-specific issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants