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

Add Rule widget #2982

Closed
rodrigogiraoserrao opened this issue Jul 22, 2023 · 13 comments · Fixed by #3209
Closed

Add Rule widget #2982

rodrigogiraoserrao opened this issue Jul 22, 2023 · 13 comments · Fixed by #3209

Comments

@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented Jul 22, 2023

This issue concerns itself with adding a new widget to Textual.
The rule widget should be similar to HTML rules.
The key difference is that HTML only has a horizontal rule and Rule's orientation can be customised ("horizontal" or "vertical").

You will want to implement a method render that is responsible for returning the characters that make up the rule itself.
You may also look at _progress_bar.py::Bar.render and _progress_bar.py::Bar.render_indeterminate for some inspiration on how to implement “a line” that you can render.

I suggest you start by implementing a horizontal rule.
After it works well, think about creating a vertical rule as well and make it customisable via a parameter, e.g., Rule(orientation="horizontal") and Rule(orientation="vertical").

Take a look at #2333 to see what files need to be changed when you implement a new widget.
Then, don't be like me and also do the final step shown in #2399 to add your new widget to the widget gallery.

Good luck and have fun!

@scriptogre
Copy link

Working on it! 😎

scriptogre pushed a commit to scriptogre/textual that referenced this issue Aug 4, 2023
- Added a new file `_rules.py` in the `src/textual/widgets` directory to define the base `Rule` widget and the `HorizontalRule` and `VerticalRule` widgets.
- Modified the `__init__.py` and `__init__.pyi` files in the `src/textual/widgets` directory to import and include the `HorizontalRule` and `VerticalRule` widgets.

The purpose of these changes is to provide support for the `HorizontalRule` and `VerticalRule` widgets in the `textual` library. These widgets allow for the creation of horizontal and vertical lines, similar to the `<hr>` HTML tag.

Related to Textualize#2982
@rodrigogiraoserrao
Copy link
Contributor Author

Hey Chris, we exchanged a couple of messages in private but I'd like to reiterate my question: are you working on this at your own pace or are you dropping this?
I'm asking just so we can know whether we'll need to tackle this ourselves or not.

@rodrigogiraoserrao
Copy link
Contributor Author

This is up for grabs.
See #3060 for a pretty good PR that is almost there.

@TomJGooding
Copy link
Contributor

TomJGooding commented Aug 29, 2023

I'm not necessarily volunteering, but just to clarify do you think this is better separated into HorizontalRule and VerticalRule widgets, or stick to the original brief of just a single Rule widget with an orientation parameter?

@willmcgugan
Copy link
Collaborator

I have no strong opinion on that. Surprising, coming from me. 🤷‍♂️

@rodrigogiraoserrao
Copy link
Contributor Author

How 'bout Rule(..., orientation=...) but then, like the Button variants, maybe provide Rule.horizontal() and Rule.vertical() class methods?

@TomJGooding
Copy link
Contributor

Ugh, apparently my fork now has snapshot tests failing on main even if I did want to volunteer. Any suggestions?

@willmcgugan
Copy link
Collaborator

How 'bout Rule(..., orientation=...) but then, like the Button variants, maybe provide Rule.horizontal() and Rule.vertical() class methods?

Sounds like the best of both worlds

@willmcgugan
Copy link
Collaborator

Ugh, apparently my fork now has snapshot tests failing on main even if I did want to volunteer. Any suggestions?

We have a few flaky tests that are timing sensitive. They run lovely but occasionally fail in CI. You may find you can run them again and they will pass.

@rodrigogiraoserrao
Copy link
Contributor Author

Ugh, apparently my fork now has snapshot tests failing on main even if I did want to volunteer. Any suggestions?

(Not sure what you are forking, but in case you are forking the PR that has some work on this already, do note that the OP had an issue in his code.)

@TomJGooding
Copy link
Contributor

TomJGooding commented Aug 29, 2023

Sorry to clarify the tests are failing locally on the same fork I've had kicking around for a while. AFAIR everything was working fine recently, perhaps due to some changes with textual-dev...?

EDIT: All tests now passing after running poetry install, though not sure what changed to see failing tests! Sorry, blame my unfamiliarity with poetry...

@willmcgugan
Copy link
Collaborator

Ah. Probably the textual-dev dependency!

@github-actions
Copy link

github-actions bot commented Sep 4, 2023

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

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 a pull request may close this issue.

4 participants