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: add some lines of documentation to clear doubts #38

Closed
wants to merge 1 commit into from

Conversation

orhun
Copy link
Sponsor Member

@orhun orhun commented Feb 12, 2023

Upstream: #677

Description

It is a PR just to resolve issue #352

Add a few lines to explain how to define the coordinates

Testing guidelines

Just test it using VS Code

Checklist

@mindoodoo mindoodoo added the documentation Improvements or additions to documentation label Feb 14, 2023
Copy link

@Nickiel12 Nickiel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it feels a little bit wordy, it will be a lot clearer for those not familiar with the ui design.

@mindoodoo
Copy link
Member

The way this PR defines X and Y is very unclear imo.

Also, looking at the original issue, it would seem like the main difference between OPs perception of the x and y coordinate system and the actual tui implementation is only that the origin is in the top left instead of bottom left.

@Nickiel12
Copy link

Maybe just a note somewhere prominent that "The origin is in the upper left corner for all widgets"?

@mindoodoo mindoodoo added this to the First Release milestone Feb 19, 2023
@mindoodoo
Copy link
Member

I honestly feel like it's pretty common sense that the origin is in the top left corner in tui applications. It also feels weird to specify it in this small case where as I'm sure there's dozens if not hundreds of places we could specify this in this crate.

Copy link
Member

@sayanarijit sayanarijit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added personal opinions.

Comment on lines +378 to +382
/// The X Axis represents the count of columns of the Rect.
///
/// The Y Axis represents the count of rows of the Rect.
///
/// The Rect is always drawn from the top left to the bottom and to the right of the container.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these docs are needed since the fields are/will be already documented.

Comment on lines +8 to +11
/// The X Axis represents the count of columns of the Line.
///
/// The Y Axis represents the count of rows of the Line.
///
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be above the respective fields.

Comment on lines +12 to +14
/// The Line is always drawn from the top left to the bottom and to the right of the container.
///
/// And when it is drawn, it will calculate the distance between the two points
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These docs aren't necessary, since there's nothing unexpected here.

@mindoodoo
Copy link
Member

If I can get a couple upvotes on this message, I'll close this PR I think. We can take another look at it if someone requests it again.

@mindoodoo
Copy link
Member

Alright, closing this then, if someone stumbles on this and disagrees / would like to discuss it further, feel free to open a discussion.

@mindoodoo mindoodoo closed this Feb 25, 2023
@joshka joshka deleted the 677/master branch July 4, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants