Skip to content

pod ui improve #59

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

Closed
wants to merge 0 commits into from
Closed

pod ui improve #59

wants to merge 0 commits into from

Conversation

nibilin33
Copy link
Contributor

No description provided.

@nibilin33 nibilin33 marked this pull request as draft November 18, 2022 15:41
@forrestbao forrestbao requested a review from lihebi November 18, 2022 17:22
ui/src/index.css Outdated
box-sizing: border-box;
padding: 10px 0;
}
.monaco .monaco-editor-background, .margin{

Choose a reason for hiding this comment

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

Copy link
Collaborator

@lihebi lihebi left a comment

Choose a reason for hiding this comment

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

Thanks!

I prefer not to use separate css files and classNames, because

  1. classNames are typically defined once and used once in React components , no need to give it a name
  2. I want the css to be close to the React component that uses it. Cross reference is painful.
  3. CSS cannot have variables, but MUI’s sx property can.

So, let’s use sx property of MUI component https://mui.com/system/getting-started/the-sx-prop/

Also, I see no need to use other styling libraries (e.g., styled components) other than MUI. The sx property of MUI seems to be implemented based on styled component already.

@@ -0,0 +1,27 @@
.result-status__success{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let’s in-line these styles into the sx property of the React component to avoid creating a new className.

ui/src/index.css Outdated
@@ -10,4 +10,8 @@ body {
code {
font-family: source-code-pro, Menlo, Monaco, Consolas, 'Courier New',
monospace;
}
.react-monaco-editor-container {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused styles.

@nibilin33 nibilin33 closed this Nov 19, 2022
@lihebi lihebi mentioned this pull request Nov 20, 2022
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