Centralize code that deals with ideal tool view actions in one class
ClosedPublic
Actions

Authored by apol on Oct 26 2016, 4:14 PM.

Details

Reviewers

brauch
kfunk

Group Reviewers

KDevelop

Commits

R33:bc4f29cb9ec6: Centralize code that deals with ideal tool view actions in one class
R32:bc4f29cb9ec6: Centralize code that deals with ideal tool view actions in one class

Summary

Make it possible for to detect if the toolview widget is disabled

In such case, we show an asterisk by the button. We could do other
things such as changing the icon or such, but trying to minimize the noise.
Changing the tooltip accordingly could also make sense.

Test Plan

Manual testing, seems to just work

Diff Detail

Repository

R33 KDevPlatform

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

apol updated this revision to Diff 7680.Oct 26 2016, 4:14 PM

apol retitled this revision from to Centralize code that deals with ideal tool view actions in one class.

apol updated this object.

apol edited the test plan for this revision. (Show Details)

apol added a reviewer: KDevelop.

Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptOct 26 2016, 4:14 PM

Looks good to me.

This revision is now accepted and ready to land.Oct 26 2016, 4:18 PM

antonanikin added a subscriber: antonanikin.Oct 26 2016, 4:22 PM

antonanikin added inline comments.

sublime/idealbuttonbarwidget.cpp
37	Remove this?

kfunk requested changes to this revision.Oct 26 2016, 5:51 PM

kfunk added a reviewer: kfunk.

kfunk added a subscriber: kfunk.

kfunk added inline comments.

sublime/idealbuttonbarwidget.cpp
43	Nitpick: `{` in next line
60	Nitpick: `{` in next line
75	Nitpick: `{` in next line
81	Hmm, not sure I like the asterisk here. To me, an asterisk usually means: "This item was modified". Can we find something better? Maybe use the disabled-mode of the icon (cf. `QIcon::Mode`)?
203	`_widgetAction` -> `widgetAction`
205	`auto toolViewAction = ...`
234–235	Dito
290	Dito

This revision now requires changes to proceed.Oct 26 2016, 5:51 PM

Use () instead of an *
Remove the action to button hash

Fixed issues.

LGTM, way better than the asterisk.

debugger/framestack/framestackwidget.cpp
257 ↗	(On Diff #7685)	The initial state of that tool view is wrong; maybe needs a `setEnabled(false)` in the ctor?

This revision is now accepted and ready to land.Oct 27 2016, 7:29 AM

Closed by commit R33:bc4f29cb9ec6: Centralize code that deals with ideal tool view actions in one class (authored by apol). · Explain WhyOct 27 2016, 10:30 AM

This revision was automatically updated to reflect the committed changes.

antonanikin mentioned this in D4219: Fix bug 374921: Tool view grouping no longer works in 5.1 branch.Jan 20 2017, 5:21 AM

antonanikin mentioned this in R33:38fa47e9860d: Fix bug 374921: Tool view grouping no longer works in 5.1 branch.Jan 23 2017, 5:07 AM

antonanikin mentioned this in R32:38fa47e9860d: Fix bug 374921: Tool view grouping no longer works in 5.1 branch.Aug 8 2017, 5:08 PM

Revision Contents
Changeset List

			Path	Packages
M			debugger/framestack/framestackwidget.cpp (3 lines)
M			sublime/idealbuttonbarwidget.h (5 lines)
M			sublime/idealbuttonbarwidget.cpp (133 lines)
M			sublime/idealcontroller.cpp (8 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	7680	3c559df		Oct 26 2016, 4:14 PM	★	★
Diff 2	7685	54cc74c	Use () instead of an *	Oct 26 2016, 11:13 PM	★	★
Diff 3	7693	54cc74c	rKDEVPLATFORMbc4f29cb9ec65c82360daf14dd0b8aa6268c0fd6	Oct 27 2016, 10:30 AM	★	★