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

Change in-editor documentation style to match online docs. #38917

Merged
merged 1 commit into from
Jun 16, 2020

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented May 21, 2020

Some minor in-editor documentation style changes:

  • Adds bullet to the property, constant, enum and method descriptions.
    consts_props
  • Rearranges some newlines.
    enums
  • Fixes inheritance and enum line font size mismatch.
    inh

@YuriSizov
Copy link
Contributor

Yay for consistency! But what's up with the enum keyword on the second one? Is it that small on purpose?

@bruvzg
Copy link
Member Author

bruvzg commented May 21, 2020

Is it that small on purpose?

No idea, I have not changed it, it was always small for me. If it's intended to be small, it's probably HiDPI scaling issue (and wrong inheritance size might be the same issue).

@YuriSizov
Copy link
Contributor

Probably is a HiDPI issue, looks fine to me:

image
image

@bruvzg bruvzg force-pushed the in_editor_docs_style branch from 6d839b3 to abb5713 Compare May 21, 2020 10:40
@bruvzg
Copy link
Member Author

bruvzg commented May 21, 2020

Probably is a HiDPI issue, looks fine to me

Yep, it's using default font instead of docs, updated PR.

@Calinou
Copy link
Member

Calinou commented May 21, 2020

Looks good to me 🙂

For extra consistency, the (em dash) character could be used to separate enum/constant values and their descriptions. If it's too long, then (en dash) can be used instead.

@bruvzg
Copy link
Member Author

bruvzg commented May 21, 2020

Update: Added cell border/background color support to the RichTextLabel (disable by default), and some table borders to the documentation (overview and properties, as the separate commit).

tables
props

For extra consistency, the — (em dash) character could be used to separate enum/constant values and their descriptions. If it's too long, then – (en dash) can be used instead.

Changed it to en dash.

@bruvzg bruvzg force-pushed the in_editor_docs_style branch from abb5713 to b8c289d Compare May 21, 2020 13:50
@bruvzg bruvzg requested a review from a team as a code owner May 21, 2020 13:50
@YuriSizov
Copy link
Contributor

YuriSizov commented May 21, 2020

table borders to the documentation (overview and properties, as the separate commit)

I like the idea, but from your screenshots on my display it's barely noticeable.
Also, some cells clearly need their inner padding adjusted. Like the ones with the type.

@bruvzg bruvzg force-pushed the in_editor_docs_style branch from b8c289d to f922016 Compare May 21, 2020 15:01
@bruvzg
Copy link
Member Author

bruvzg commented May 21, 2020

Added some padding (and cell padding support to the RichTextLabel), and changed colors to the editor theme default background variants.

def_themes

@bruvzg bruvzg force-pushed the in_editor_docs_style branch from f922016 to f093944 Compare May 21, 2020 16:31

ItemFrame() {
type = ITEM_FRAME;
parent_frame = nullptr;
cell = false;
parent_line = 0;
odd_row_bg = Color(0, 0, 0, 0);
Copy link
Member

@akien-mga akien-mga May 22, 2020

Choose a reason for hiding this comment

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

Move initialization to the declaration.

Edit: Fixed typo.

@akien-mga
Copy link
Member

While the added RTL features look nice, I think they might warrant a dedicated PR as it's a core change, and it will conflict with #39248.
The rest of the doc style changes should be mergeable without second thought already.

@bruvzg
Copy link
Member Author

bruvzg commented Jun 16, 2020

I think they might warrant a dedicated PR as it's a core change

Done, RTL related changes are in #39587

@akien-mga akien-mga merged commit 5ce02b1 into godotengine:master Jun 16, 2020
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants