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

refactor(example): improve constraints and flex examples #817

Merged
merged 25 commits into from
Jan 16, 2024

Conversation

Valentin271
Copy link
Member

@Valentin271 Valentin271 commented Jan 14, 2024

This PR is a follow up to #811.

It improves the UI of the layouts by

  • thoughtful accessible color that represent priority in constraints resolving
  • using QUADRANT_OUTSIDE symbol set for block rendering
  • adding a scrollbar
  • panic handling
  • refactoring for readability

to name a few. Here are some example gifs of the outcome:

constraints

flex

@kdheepak
Copy link
Collaborator

Looks great! Thanks for making the changes!

@joshka
Copy link
Member

joshka commented Jan 14, 2024

Could it be worth putting the width bar at the top / bottom and compressing the examples into half as many lines?

@kdheepak
Copy link
Collaborator

I tried it and I think it does look better. I ended up making the width bar sticky and only at the top. Scroll doesn't affect it so we don't need it at the bottom.

image

Copy link

codecov bot commented Jan 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cc6737b) 92.4% compared to head (1b2ae09) 92.5%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff           @@
##            main    #817    +/-   ##
======================================
  Coverage   92.4%   92.5%            
======================================
  Files         59      59            
  Lines      15616   15745   +129     
======================================
+ Hits       14437   14571   +134     
+ Misses      1179    1174     -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kdheepak
Copy link
Collaborator

Here's what it looks like with the flex examples

image

@kdheepak
Copy link
Collaborator

Here's what it looks like with a one line spacer:

image

@kdheepak
Copy link
Collaborator

Once this is merged I'll make a documentation PR to update the code snippets with this style.

@joshka
Copy link
Member

joshka commented Jan 14, 2024

what about putting the pixels inside and the constraints on the border?

@kdheepak
Copy link
Collaborator

what about putting the pixels inside and the constraints on the border?

Interesting idea.

image

I didn't like it as much because the constraint types don't stand out. But changing the colors makes it better I think:

image

@kdheepak
Copy link
Collaborator

I like this better:

image

@joshka
Copy link
Member

joshka commented Jan 15, 2024

What about the second option, but with centered headings?

@joshka
Copy link
Member

joshka commented Jan 15, 2024

Using background colors instead of borders looks pretty nice:

Made with VHS

Though constraints that match and are next to each other need an indicator

@joshka
Copy link
Member

joshka commented Jan 15, 2024

image

The underline is really difficult to see here.

@kdheepak
Copy link
Collaborator

I considered it but in addition to what you said I also want something that is copy paste friendly, e.g.:

┌Flex Layouts ─ Use h l or ◄ ► to change tab and j k or ▲ ▼  to scroll─────────┐
│ Stretch │ StretchLast │ Start │ Center │ End │ SpaceAround │ SpaceBetween    │
└──────────────────────────────────────────────────────────────────────────────┘

<------------------------------------80 px------------------------------------->

┌─────────Length(20)─────────┐┌────Fixed(20)─────┐┌───────Percentage(20)───────┐
│            30 px           ││       20 px      ││            30 px           │
└────────────────────────────┘└──────────────────┘└────────────────────────────┘

┌────Fixed(20)─────┐┌───────Percentage(20)───────┐┌─────────Length(20)─────────┐
│       20 px      ││            30 px           ││            30 px           │
└──────────────────┘└────────────────────────────┘└────────────────────────────┘

@joshka
Copy link
Member

joshka commented Jan 15, 2024

@joshka
Copy link
Member

joshka commented Jan 15, 2024

Copied...

▐              Fixed(40)               ▌▐              Proportional(0)              ▌
▐                 40 px                ▌▐                   45 px                   ▌

▐    Fixed(20)     ▌▐    Fixed(20)     ▌▐              Proportional(0)              ▌
▐       20 px      ▌▐       20 px      ▌▐                   45 px                   ▌

@joshka
Copy link
Member

joshka commented Jan 15, 2024

Lots of little things changed in those refactoring changes:

  • color eyre
  • moved examples to a const
  • made height calcs a method
  • moved app state / running stuff around, so the main call is just App::default().run(terminal)?;
  • don't render the spacers when the description is empty
  • tailwind colors for tabs, highlighted is reverse
  • tabs and axis methods just return impl Widget and let the render method call it.
  • render_demo creates a buffer the same size of the area and renders into it and then splices that into the main buffer
  • next/prev tab functions use strum::from_repr
  • renamed exampleSelection to SelectedTab
  • SelectedTab to_tab_title uses strum::Display to get to_string()
  • dynamically calculate example heights
  • simplify the Example widget::render and illustration methods
  • move color for constraint to a method

@joshka
Copy link
Member

joshka commented Jan 15, 2024

image
image

@Valentin271
Copy link
Member Author

I like where this is going. I've just added some movement keybinding and adjusted the display on constraints to show the pixels.

I think the only thing left is to reorder the code in constraints right?

@orhun
Copy link
Member

orhun commented Jan 15, 2024

Damn, that new flex example is so good!

@joshka
Copy link
Member

joshka commented Jan 15, 2024

I think the only thing left is to reorder the code in constraints right?

@kdheepak was going to add comments to the examples. I think it could do with a ScrollBar as well.

I'll reorder the constraints example to match flex, and add the ScrollBar shortly.

Damn, that new flex example is so good!

A joint effort of bouncing ideas back and forth with @kdheepak last night :)

@Valentin271
Copy link
Member Author

Alright. Agree for the scrollbar if you can make it work without too much hassle. I couldn't really because of the hacky way I implemented the scroll.

@joshka
Copy link
Member

joshka commented Jan 16, 2024

constraints

flex

Support multi-line comments
Inverse tab colors
Co-authored-by: Josh McKinney <[email protected]>
@joshka
Copy link
Member

joshka commented Jan 16, 2024

Can we summarize this change in the first comment and then squash it?

@kdheepak
Copy link
Collaborator

I've updated the PR description. This should be ready to squash. @joshka you want to do the honors?

@joshka joshka changed the title refactor(example): add scroll to constraints example refactor(example): improve constraints and flex examples Jan 16, 2024
@joshka joshka merged commit 813f707 into main Jan 16, 2024
42 checks passed
@joshka joshka deleted the refactor/scroll-constraints-example branch January 16, 2024 04:56
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.

4 participants