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

Css changes #913

Merged
merged 3 commits into from
Nov 7, 2024
Merged

Css changes #913

merged 3 commits into from
Nov 7, 2024

Conversation

rahulkgupta
Copy link
Collaborator

CleanShot 2024-11-06 at 16 23 19

  • use tailwind
  • consolidate toolbar component
  • move toolbar up
  • move the panels down so that they dont overlap with toolbar buttons
  • move undo and redo away from being below the save button so that the save buttons dont overlap with the undo

potentially negative side effects

  • remove loading logo. got too complex to line it up with the tailwind logo
  • hover state on the right panel doesn't quite work at the moment. I dont know if it's honestly that important, but it was too much to figure it out this round given some of the shared classes between the left and right panel. i can tailwindify the side panels down the line

@kfarr kfarr merged commit fc46b4a into main Nov 7, 2024
1 check passed
@kfarr kfarr deleted the css-changes branch November 7, 2024 23:30
"emulator": "npm run dist:staging && npm run prefirebase && cd public && firebase emulators:start"
"emulator": "npm run dist:staging && npm run prefirebase && cd public && firebase emulators:start",
"build:css": "tailwindcss -i ./src/styles/tailwind.css -o ./dist/tailwind.css",
"watch:css": "tailwindcss -i ./src/styles/tailwind.css -o ./dist/tailwind.css --watch"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You shouldn't need the tailwindcss cli if you're using postcss-loader in the webpack config.

@@ -9,7 +9,7 @@ import styles from './Logo.module.scss';
* @category Components
*/
const Logo = ({ onToggleEdit, isEditor }) => (
<div className={styles.wrapper}>
<div className="flex items-center space-x-2">
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you use display flex, prefer using gap-2 instead of space-x-2 that uses margins on children.

@@ -335,14 +335,12 @@ export default class SceneGraph extends React.Component {
// Outliner class names.
const className = classNames({
outliner: true,
hide: this.state.leftBarHide
hide: this.state.leftBarHide,
'mt-16': true
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a general note, avoid margin as much as possible, even forbid it, use padding instead if you want components to be reusable. Space between elements should be set by the parent element on its children (via flex gap for example). Here you should really modify the top value for the outliner class instead of adding a margin.

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