search

enricozb / intuitive

A library for building declarative text-based user interfaces
213 stars 3 forks source link

Add alignment property to Text #6

Open chrisbouchard opened 1 year ago

chrisbouchard commented 1 year ago

This merge request introduces an alignment property to the Text component, and a new Alignment enum with alignment options (left, center, right).

Breaking Change

This is a breaking change in that Text::new now takes an additional parameter — since that would be a breaking change no matter what, I added alignment at the beginning of render's parameters to preserve alphabetical order.

Implementation

Under the covers, we directly translate the Alignment to a tui-rs Alignment and set it onto the underlying Paragraph. The default is Alignment::Left, so there should be no change in default behavior.

I also refactored the components::text module a bit, so that it could become a public module (to expose Alignment) without double-exposing the Text component — I followed the pattern from components::stack of creating a pub(super) submodule to hide the component. I couldn't think of a good submodule name, so I used component. I considered using the component name “text,” but components::text::text seemed more confusing than helpful. I'm open to any suggestions, including using “text” if you prefer;

Fixes #5.

chrisbouchard commented 1 year ago

I could also see putting Alignment somewhere else, e.g., if it might some day also be used to align the title on Section.

enricozb commented 1 year ago

Thank you for the PR.

I'm actually in the process of rewriting intuitive completely because of a fundamental flaw (see https://github.com/enricozb/intuitive/issues/4). This rewrite that is in progress also removes tui-rs as a dependency in favor of a custom drawing solution. Furthermore, this re-rewrite will also pave the way for a focusing system (see https://github.com/enricozb/intuitive/issues/3) and precise re-renders using signals.

Your suggestion to add Alignment to Text, and possibly Section, is a really good one, and I'll consider adding it to the rewrite as well. I'm trying to have it close to feature parity with v0.6 before pushing any code.

Let me think a bit about merging this PR, since so much will change in the upcoming v0.7 anyways.