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.

Sign up for GitHub

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

Jump to bottom

Add support for italic tab titles & italicise unsaved scene tabs #88709

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for italic tab titles & italicise unsaved scene tabs #88709

wants to merge 1 commit into from
+76 −21

Conversation

Cass-dev-web
Copy link
Contributor

@Cass-dev-web Cass-dev-web commented Feb 23, 2024 •
edited by Mickeon
Loading

Bugsquad: Closes godotengine/godot-proposals#9161 .

This PR [derived from proposition]:

  • Adds support to vary the style of the tab titles on the group TabBar::Tab using the new enum TabBar::TabTitleStyle.
  • Makes any unsaved scene tab have its title italicised for clearer communication with the user

PS: There's room for improvement when it comes to performance. See Line 343 of scene/gui/tab_bar.cpp

@Cass-dev-web Cass-dev-web requested review from a team as code owners February 23, 2024 14:45
@Cass-dev-web
Copy link
Contributor Author

I am currently fighting with git to squash these changes, bear with me...

@Cass-dev-web Cass-dev-web force-pushed the master branch 2 times, most recently from 77df04b to 63c0bac Compare February 23, 2024 15:27
@YeldhamDev YeldhamDev added this to the 4.x milestone Feb 23, 2024
@Calinou
Copy link
Member

Calinou commented Feb 23, 2024

Thanks for opening a pull request 🙂

Feature pull requests should be associated to a feature proposal to justify the need for implementing the feature in Godot core. Please open a proposal on the Godot proposals repository (and link to this pull request in the proposal's body).

@Cass-dev-web
Copy link
Contributor Author

Git has been rebased successfully 👍

@Cass-dev-web Cass-dev-web mentioned this pull request Feb 23, 2024
Open
@KoBeWi
Copy link
Member

KoBeWi commented Feb 23, 2024

Hard-coding font style is a questionable solution. It would be better if TabBar had theme font styles for normal and italic, like in RichTextLabel. Editor theme has such fonts so you'd get them for free, not sure about default theme. It would probably fall on the user to provide an italic font.

@Cass-dev-web
Copy link
Contributor Author

Hard-coding font style is a questionable solution. It would be better if TabBar had theme font styles for normal and italic, like in RichTextLabel. Editor theme has such fonts so you'd get them for free, not sure about default theme. It would probably fall on the user to provide an italic font.

After testing a bit, I just noticed that TabBar and TabBar::Tab are indeed nodes creatable by the user. I'll see what I can do about it.

@Cass-dev-web Cass-dev-web marked this pull request as draft February 24, 2024 13:15
@Cass-dev-web
Copy link
Contributor Author

Cass-dev-web commented Feb 24, 2024 •
edited
Loading

After consideration, I feel like the property should stay as an internal one until someone is able to convert these changes into the node.

@Cass-dev-web Cass-dev-web marked this pull request as ready for review February 24, 2024 16:33
bruvzg
bruvzg reviewed Feb 26, 2024
View reviewed changes
editor/editor_node.cpp Outdated
@@ -7361,7 +7361,7 @@ EditorNode::EditorNode() {
vcs_actions_menu = VersionControlEditorPlugin::get_singleton()->get_version_control_actions_panel();
vcs_actions_menu->set_name("Version Control");
vcs_actions_menu->connect("index_pressed", callable_mp(this, &EditorNode::_version_control_menu_option));
vcs_actions_menu->add_item(TTR("Create Version Control Metadata..."), RUN_VCS_METADATA);
vcs_actions_menu->add_item(TTR("Create/Override Version Control Metadata..."), RUN_VCS_METADATA);
Copy link
Member

Choose a reason for hiding this comment

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

Seems to be unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes probably messed up somewhere with git.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is part of #88609

@Cass-dev-web
Copy link
Contributor Author

Currently fixing all issues associated with the changes...

@Cass-dev-web Cass-dev-web requested a review from a team as a code owner February 26, 2024 11:37
@Mickeon
Copy link
Contributor

Mickeon commented Feb 26, 2024

I don't know how to feel about having an option for them to be drawn specifically in italics.
Then in the future someone may suggest "Hey, how about bold?", "How about outlines?" or "What about colors?" and the solution in this PR is not exactly scalable.
Of course, just because it says "Italics" doesn't mean you have to provide an italics font, but it does not feel right.

The ability to override the font per Tab sounds more enticing to me, personally.

Mickeon
Mickeon reviewed Feb 26, 2024
View reviewed changes
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Docs are completely fine, but my sentiment above still stands.

@Cass-dev-web
Copy link
Contributor Author

I don't know how to feel about having an option for them to be drawn specifically in italics.

The purpose of this PR is mainly to build foundation on which other contributors can expand their ideas upon. This gives the user and the source code contributors more control over the styling of each scene tab, making it a more polished experience for the user in general in the long term run, but I understand where your ideas come from, albeit, I don't have an issue with also adding the feature for a bold style in the future.

@Mickeon
Copy link
Contributor

Mickeon commented Feb 26, 2024

Problem is, italics_font and italics_font_size Theme items are both available in the public API in this PR as is, which means they would have to be maintained as developers in Godot can make use of it, too. If this implementation is later changed, these items would then become superfluous. Deprecating can be afforded but it's worth being a bit careful.

@Cass-dev-web
Copy link
Contributor Author

Squashed all commits.

@Cass-dev-web Cass-dev-web requested a review from bruvzg February 27, 2024 10:36
@kitbdev kitbdev mentioned this pull request Oct 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mickeon Mickeon Mickeon left review comments

@bruvzg bruvzg Awaiting requested review from bruvzg

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
enhancement topic:editor topic:gui
Projects
None yet
Milestone
4.x
Development

Successfully merging this pull request may close these issues.

Adding support for styling the text on TabBar Tabs
6 participants
@Cass-dev-web @Calinou @KoBeWi @Mickeon @bruvzg @YeldhamDev