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

feat: new UI and refactored styles #101

Closed
wants to merge 16 commits into from
Closed

feat: new UI and refactored styles #101

wants to merge 16 commits into from

Conversation

vinpac
Copy link

@vinpac vinpac commented Apr 15, 2022

Proposal
Rewrite styles so Ladle gets a Fresh and Professional UI that reflects its engineering powers.

Screenshots
Example

Considerable Changes

  • Compile styles using Postcss, CSS Nano, Autoprefixer and Tailwind
  • Split Tree View components for better maintainability
  • Add a Script that gathers all SVG Icons and creates an icons.tsx file exporting all of them. Here's an example:
export const Maximize: React.FC<React.SVGProps<SVGSVGElement>> = (props) => (
  <svg
     ...

Tasks

  • Support Dark Mode
  • Use CSS Variables for Colors
  • Support changing the Sidebar Position
  • Support RTL
  • Add Mobile Styles
  • Style Components
    • Text Input Control
    • Select Input Control
    • Checkbox Input Control
    • Boolean Input Control
    • Radio Input Control
    • Addon Panel
    • Story Title
    • Tooltip
    • Mobile Sidebar Navigation
    • Search
    • Tree Stories
    • Tree Components
    • Tree Folders
    • Search

@vinpac vinpac changed the title Refactor Styles feat: new UI and refactored styles Apr 15, 2022
@vinpac vinpac mentioned this pull request Apr 16, 2022
8 tasks
@jcleefw
Copy link
Contributor

jcleefw commented Jul 8, 2022

hi @vinpac Just wonder how are you going and planning to complete this PR? Would you like some help move forward?

@benvds
Copy link

benvds commented Jul 13, 2022

hi @vinpac Just wonder how are you going and planning to complete this PR? Would you like some help move forward?

Maybe separate the mobile and sidebar position tasks into another PR? Otherwise I'd also like to help getting this done.

@vinpac
Copy link
Author

vinpac commented Jul 13, 2022

Hey guys @jcleefw @benvds

Sorry for the lack of updates on this one. I had some personal problems and it was hard to focus on this.

I would love a hand on this. Could you guys help me?

@jcleefw
Copy link
Contributor

jcleefw commented Jul 15, 2022

Hey guys @jcleefw @benvds

Sorry for the lack of updates on this one. I had some personal problems and it was hard to focus on this.

I would love a hand on this. Could you guys help me?

Happy to work on the Sidebar position. But not too sure how the Sidebar position is suppose to be since the RTL support does change the position of the Sidebar.

@lucasruy
Copy link

If you need help I'm available to contribute too 😄
I would be very happy to see this PR being finalized

@benvds
Copy link

benvds commented Jul 29, 2022

sorry for the late reply, didn't get the notification.

If we separate the mobile and sidebar position tasks into another PR it's easier for use to split up the work while also getting this thing moving.

@tajo what do u think?

@tajo
Copy link
Owner

tajo commented Jul 29, 2022

I guess you can create a branch and target that for multiple PRs but it should be release altogether right?

@changeset-bot
Copy link

changeset-bot bot commented Jul 31, 2022

⚠️ No Changeset found

Latest commit: 1593c18

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jul 31, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
ladle ❌ Failed (Inspect) Jul 31, 2022 at 9:19PM (UTC)

@npearson72
Copy link

Any chance we can have an option to move it to the left side of the screen?

@jcleefw
Copy link
Contributor

jcleefw commented Sep 1, 2022

Given that this branch is quite behind now and it's hard to keep in contact @vinpac, I think the best way is to copy the work done by @vinpac to another fork branch and move forward. I'll probably start looking into this.

Here's the current plan of attack I'm proposing. Each line can probably be it's own PR.

  • setup v2 styling folder so that we can have incremental changes while allowing multiple forks
  • enable styling libraries, postcss and tailwind, add basic folder structure, setup CSS Variables for Colors
  • update icons according to @vinpac commits
  • Split & refactor tree view
  • Style search component, main and addon panels
  • Style story
  • Style controls & tooltip
  • Support Dark Mode
  • Support RTL

@tajo Here's a few things I need to clarify before moving ahead

  1. is PostCSS & tailwindcss the CSS direction you're happy to proceed? Personally, I'm not familiar with It but i think @vinpac has done an excellent job for anyone to pick up from then on. So I'm easy with anything
  2. The current setup sidebar is on the Right by default, is there any preference on left or right?
  3. What's the best way to approach this by batch merging? I assume you have controls on what gets published in what version so batch merging to your repo should still be alright?

I think the critical things will be the first 2 of the check list and then we can all start working in parallel.

@benvds @lucasruy Still up for working on this? Shall we keep in contact on discord?

@tajo
Copy link
Owner

tajo commented Sep 1, 2022

is PostCSS & tailwindcss the CSS direction you're happy to proceed? Personally, I'm not familiar with It but i think @vinpac has done an excellent job for anyone to pick up from then on. So I'm easy with anything

I think it's a good setup so don't mind using those tools.

The current setup sidebar is on the Right by default, is there any preference on left or right?

Personally I prefer the right sidebar since it makes easier to test a11y when it comes to keyboard navigation - the first focusable part of the UI is the story. Sure you can change the order and other workarounds but it just makes sense spatially. This could be also a setting so everyone can change it.

What's the best way to approach this by batch merging? I assume you have controls on what gets published in what version so batch merging to your repo should still be alright?

It can't be merged to the main branch without being published or blocking future releases. So it should be a separate branch. There should not be that many merge conflicts since its focused on the UI and we haven't been making that many changes there recently.

Other approach would be a feature flag but that's a risk if there is no strong commitment to actually finish it - it would just mess up the git history and codebase.

@lucasruy
Copy link

@benvds @lucasruy Still up for working on this? Shall we keep in contact on discord?

@jcleefw Yes, I can help you finish this job. Just show me where to start and what to do and I can start working on it in my spare time.

@k-ruben
Copy link

k-ruben commented Sep 17, 2022

Would love to see work done on the UI Part. Without any right to ask for features, I would do the sidebar similar to the dev-tool in browsers with a right and bottom option being the important ones and a new window being a nice to have which I probably will be doing research on myself.

@tajo
Copy link
Owner

tajo commented Feb 5, 2023

Closing for now since there's not been much activity. Feel free to re-open / continue.

@tajo tajo closed this Feb 5, 2023
@maksimf
Copy link

maksimf commented Jun 1, 2023

@vinpac do you have any plans to finish this? Or maybe we could've have it merged as is w/o mobile version? Looks great, and it seems like 85% of work is done

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.

None yet

8 participants