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

docs(examples): Update block example #351

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Conversation

joshka
Copy link
Member

@joshka joshka commented Jul 27, 2023

Block example

@joshka
Copy link
Member Author

joshka commented Jul 28, 2023

I have several goals in re-doing this example:

  • to create consistency the the setup / restore code, so we can document that once for all the examples
  • to use similar styles, colors, sizing, etc. across widgets so it looks like a harmonious set of examples
  • to show off every feature of the widget (a method per block makes it easy to see which code draws which block)
  • to easily see catch visual regressions (e.g. this example highlights the title style bug)
  • to be able to see visually how any PRs change the display of a widget

My hope is that by starting with a single easy example and then doing a more complex one, we have a couple of high quality examples that we can point at and then make the other widgets consistent with (e.g. add some issues labelled good first issue). Keeping the example code super simple and obvious makes it easy to update when we add / change functionality.

@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #351 (52c660a) into main (b9290b3) will decrease coverage by 0.33%.
Report is 9 commits behind head on main.
The diff coverage is 57.84%.

@@            Coverage Diff             @@
##             main     #351      +/-   ##
==========================================
- Coverage   85.08%   84.76%   -0.33%     
==========================================
  Files          40       40              
  Lines        8608     8603       -5     
==========================================
- Hits         7324     7292      -32     
- Misses       1284     1311      +27     
Files Changed Coverage Δ
src/backend/crossterm.rs 0.00% <0.00%> (ø)
src/backend/termion.rs 27.46% <0.00%> (-0.59%) ⬇️
src/terminal.rs 55.73% <0.00%> (-0.47%) ⬇️
src/text/masked.rs 98.64% <0.00%> (ø)
src/widgets/calendar.rs 81.06% <0.00%> (-1.25%) ⬇️
src/widgets/canvas/circle.rs 97.14% <0.00%> (ø)
src/widgets/canvas/line.rs 59.67% <0.00%> (ø)
src/widgets/canvas/map.rs 0.00% <0.00%> (ø)
src/widgets/canvas/mod.rs 91.61% <0.00%> (-0.27%) ⬇️
src/widgets/canvas/points.rs 85.71% <0.00%> (+39.56%) ⬆️
... and 25 more

@kdheepak
Copy link
Collaborator

I like having an example like this!

Thoughts on making this interactive? If I could hit j and k to cycle through the various options you have here and the gif showed all cycles, that would be visually easier to grok.

Also, maybe for styled borders, only change foreground colors? (this is feedback that I think could go in a separate PR about making everything prettier)

@joshka
Copy link
Member Author

joshka commented Jul 29, 2023

Those are both good points.

I think it’s worth making interactive examples separate to the examples that demonstrate how things look. There’s a lot of ways of doing interactivity that work well small for demos but don’t scale well to apps that have more a diverse UI.

I experimented a little with having a cli flag to to choose the example to display so we could use VHS to generate smaller images to include directly in the docs, but it’s difficult to keep it simple with that approach. This example would be easy to refactor to add interactivity though, so perhaps let’s revisit the idea after this.

My rationale behind the styled borders using the background color was to make it easy to contrast the difference between the block style and border style. The background makes it obvious how the dark gray styling on the paragraph combines with the background block style. Background color also helps show which things affect blank space not just the text (which the foreground doesn’t). Do you think this makes sense?

Overall my goal is to try to pare this down to the absolute minimal amount of code necessary to show the info we need. There’s some ideas I have about fixing the backend implementations that could make this even smaller.

@joshka joshka requested a review from orhun July 29, 2023 22:46
@joshka
Copy link
Member Author

joshka commented Aug 4, 2023

Merging this to make it easy to see that #363 fixes the bug

@joshka joshka added this pull request to the merge queue Aug 4, 2023
Merged via the queue into ratatui:main with commit 554805d Aug 4, 2023
@joshka joshka deleted the block-example branch August 4, 2023 10:03
@joshka joshka added this to the v0.23.0 milestone Aug 21, 2023
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