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(stylize): allow all widgets to be styled #289

Merged
merged 2 commits into from
Jul 14, 2023

Conversation

joshka
Copy link
Member

@joshka joshka commented Jul 1, 2023

This PR:

  • renames the underlined and dimmed methods to match the Style consts (underline and dim)
  • adds Stylize implementations for all the remaining widgets
  • Implements stylized for Style enabling. E.g.: Style::new().blue()
  • Makes it possible to remove modifiers. E.g.: Style::new().not_bold()
  • Documents the changes
  • Simplifies the Stylize macros

Details:

  • Add styled impl to:
    • Barchart
    • Chart (including Axis and Dataset),
    • Guage and LineGuage
    • List and ListItem
    • Sparkline
    • Table, Row, and Cell
    • Tabs
    • Style
  • Allow modifiers to be removed (e.g. .not_italic())
  • Allow .bg() to recieve Into
  • Made shorthand methods consistent with modifier names (e.g. dim() not
    dimmed() and underlined() not underline())
  • Simplify integration tests
  • Add doc comments
  • Simplified stylize macros with https://crates.io/crates/paste

@joshka
Copy link
Member Author

joshka commented Jul 1, 2023

Pinging @samyosm for a review on this as a follow up on #283

@codecov
Copy link

codecov bot commented Jul 2, 2023

Codecov Report

Merging #289 (49152db) into main (2889c7d) will increase coverage by 1.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #289      +/-   ##
==========================================
+ Coverage   83.85%   85.00%   +1.15%     
==========================================
  Files          38       38              
  Lines        8045     8464     +419     
==========================================
+ Hits         6746     7195     +449     
+ Misses       1299     1269      -30     
Impacted Files Coverage Δ
src/style.rs 92.53% <100.00%> (+3.60%) ⬆️
src/style/stylize.rs 100.00% <100.00%> (ø)
src/text.rs 71.42% <100.00%> (+0.22%) ⬆️
src/widgets/barchart/mod.rs 89.67% <100.00%> (+0.30%) ⬆️
src/widgets/block.rs 86.39% <100.00%> (+0.29%) ⬆️
src/widgets/chart.rs 97.23% <100.00%> (+1.29%) ⬆️
src/widgets/gauge.rs 92.40% <100.00%> (+5.19%) ⬆️
src/widgets/list.rs 98.03% <100.00%> (+0.09%) ⬆️
src/widgets/paragraph.rs 99.81% <100.00%> (+1.11%) ⬆️
src/widgets/sparkline.rs 89.26% <100.00%> (+5.00%) ⬆️
... and 2 more

@orhun
Copy link
Member

orhun commented Jul 2, 2023

I thought we were putting macro code behind macros feature but that's only for user-facing changes, right?

@joshka
Copy link
Member Author

joshka commented Jul 2, 2023

I thought we were putting macro code behind macros feature but that's only for user-facing changes, right?

Yep. Just user facing macros makes sense. Here they cut down on hundreds of LoC so that’s really useful.

@joshka joshka force-pushed the feat-stylize-all-the-things branch 2 times, most recently from 5277c79 to e8a95d3 Compare July 10, 2023 23:23
@joshka
Copy link
Member Author

joshka commented Jul 10, 2023

(rebased on main)

@joshka joshka force-pushed the feat-stylize-all-the-things branch from e8a95d3 to 42d5432 Compare July 11, 2023 02:41
@joshka joshka force-pushed the feat-stylize-all-the-things branch from 42d5432 to 2b4ea4d Compare July 14, 2023 00:09
@joshka
Copy link
Member Author

joshka commented Jul 14, 2023

Rebased on main to resolve conflicts with prelude introduction and barchart changes (barchart.rs -> barchart/mod.rs).

@joshka joshka force-pushed the feat-stylize-all-the-things branch 2 times, most recently from 5373442 to b24c82e Compare July 14, 2023 00:22
@sayanarijit
Copy link
Member

This is cool...

joshka added 2 commits July 13, 2023 20:59

Verified

This commit was signed with the committer’s verified signature.
joshka Josh McKinney
- Add styled impl to:
  - Barchart
  - Chart (including Axis and Dataset),
  - Guage and LineGuage
  - List and ListItem
  - Sparkline
  - Table, Row, and Cell
  - Tabs
  - Style
- Allow modifiers to be removed (e.g. .not_italic())
- Allow .bg() to recieve Into<Color>
- Made shorthand methods consistent with modifier names (e.g. dim() not
  dimmed() and underlined() not underline())
- Simplify integration tests
- Add doc comments
- Simplified stylize macros with https://crates.io/crates/paste

Verified

This commit was signed with the committer’s verified signature.
joshka Josh McKinney
Runny clippy first means that we fail fast when there is an issue that
can easily be fixed rather than having to wait 30-40s for the failure
@joshka joshka force-pushed the feat-stylize-all-the-things branch from b24c82e to 49152db Compare July 14, 2023 04:01
@joshka
Copy link
Member Author

joshka commented Jul 14, 2023

Fixed up doc tests problem that I introduced by adding more docs.

@joshka joshka added this pull request to the merge queue Jul 14, 2023
Merged via the queue into ratatui:main with commit 9f1f59a Jul 14, 2023
@joshka joshka deleted the feat-stylize-all-the-things branch July 14, 2023 08:38
a-kenji pushed a commit to a-kenji/ratatui that referenced this pull request Jul 16, 2023
* feat(stylize): allow all widgets to be styled

- Add styled impl to:
  - Barchart
  - Chart (including Axis and Dataset),
  - Guage and LineGuage
  - List and ListItem
  - Sparkline
  - Table, Row, and Cell
  - Tabs
  - Style
- Allow modifiers to be removed (e.g. .not_italic())
- Allow .bg() to recieve Into<Color>
- Made shorthand methods consistent with modifier names (e.g. dim() not
  dimmed() and underlined() not underline())
- Simplify integration tests
- Add doc comments
- Simplified stylize macros with https://crates.io/crates/paste

* build: run clippy before tests

Runny clippy first means that we fail fast when there is an issue that
can easily be fixed rather than having to wait 30-40s for the failure
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

4 participants