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

Deprecate the ToolButton node #31518

Closed
YeldhamDev opened this issue Aug 20, 2019 · 8 comments
Closed

Deprecate the ToolButton node #31518

YeldhamDev opened this issue Aug 20, 2019 · 8 comments

Comments

@YeldhamDev
Copy link
Member

ToolButton is a Button that has flat enabled by default. That's it.

I don't think such a thing needs a node of its own, and would probably be for the better if it was marked as deprecated and then removed in 4.0.

But wait! It's heavily used in the editor itself!
Indeed, and it saves a total of one single line. Sure, the editor has lots of buttons, so those single lines pile up, but here's a better solution, making Button take a argument when created, like Label does:
Label *new_label = memnew(Label("Placing text here avoids needing a extra line."));

@KoBeWi
Copy link
Member

KoBeWi commented Aug 20, 2019

I love how single copyright header is longer than whole declaration and implementation of this class XD

Constructor argument makes sense, but is flat really that important option to be there? (I mean outside editor ofc)
EDIT: Ok, maybe this doesn't matter with the overloading.

@groud
Copy link
Member

groud commented Aug 21, 2019

Making ToolButton an editor-only feature makes even more sense IMHO.

@akien-mga
Copy link
Member

Should we simply make the editor theme have flat by default for all buttons, and drop ToolButton + the manual set_flat()s? Or are there good use cases for both flat and non-flat buttons in the editor?

@groud
Copy link
Member

groud commented Aug 21, 2019

Should we simply make the editor theme have flat by default for all buttons

That would make sense, but for now the "flat" button property is not overridable in the theme. It should be made an overridable constant.

@YeldhamDev
Copy link
Member Author

There are a lot of non-flat buttons (mainly the "Cancel" and "OK" in dialogs) that would look odd if flat, so I don't think setting all buttons to one option would work.

@groud
Copy link
Member

groud commented Aug 21, 2019

Indeed, if we move that to a theme constant, it can also be overriden later on for specific nodes.

@Calinou
Copy link
Member

Calinou commented Aug 21, 2019

For the record, the editor theme redesign I'm currently working on doesn't need the flat property, so I already removed it there. ToolButton is still present in that branch, but it's identical to a Button otherwise (because I didn't bother replacing it yet).

@YeldhamDev
Copy link
Member Author

Closed by #39690.

@YeldhamDev YeldhamDev added this to the 4.0 milestone Jun 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants