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

[Flamingo] Evolution of command button component classes #231

Closed
IanMayo opened this issue Jan 9, 2020 · 13 comments
Closed

[Flamingo] Evolution of command button component classes #231

IanMayo opened this issue Jan 9, 2020 · 13 comments
Assignees
Labels
3.0 - Diamond Release 3.0 - 2020.H1 Breaking change Marking changes that break backwards compatibility

Comments

@IanMayo
Copy link

IanMayo commented Jan 9, 2020

Version of Radiance (current development is 3.0-SNAPSHOT)

2.5.1

Sub-project (Neon, Trident, Substance, Flamingo, ...)

Flamingo

Version of Java (current minimum is 9)

11

Version of OS

MACOS/Linux

The issue you're experiencing (expected vs actual, screenshot, stack trace etc)

We'd like to subclass the Flamingo Command Toggle Button, to include a drop-down menu, like in this UX mockup:
image

But, since the constructor only has package visibility, we're not able to.

Is it an option to relax the constructor visibility?

Or do you have an alternate strategy for sub-classing a Flamingo control?

@kirill-grouchnikov
Copy link
Owner

Can you be a little more specific on the particular class? JCommandToggleButton is public and its constructor is public as well.

@IanMayo
Copy link
Author

IanMayo commented Jan 9, 2020

Sorry. It was SubstanceCommandToggleButtonUI that I was referring to.

Hmm, maybe there's an alternate strategy for adding drop-down menu functionality to a toggle button.

@IanMayo IanMayo changed the title Allow Radiance components to be extended [Flamingo] Allow Flamingo components to be sub-classed Jan 9, 2020
@kirill-grouchnikov
Copy link
Owner

Right, anything in .internal packages is not part of the API and subject to change at any time. That includes all UI delegates - from Substance and Flamingo.

But let's take a step back and talk about the specific requirement - adding drop-down menu to a toggle button. Can you point me to where this functionality is available (preferably in Office) so that I can see the mechanics behind it? Is it that the button has two area, one for action / toggle, and one for showing the popup?

@IanMayo
Copy link
Author

IanMayo commented Jan 9, 2020

Yes, the button has two areas. See the video of our implementation:
https://www.youtube.com/watch?v=jeQPCyOI8Fg

In MACOS, a toggle button is used to start/stop screen recording. A drop-down menu is used to specify the audio source for the recording:
image

Here's a microsoft article on the concept of a Split Button:
https://docs.microsoft.com/en-us/openspecs/office_standards/ms-customui/aa41c698-c7e8-4486-b15f-b73bedbf2be8

As the top video shows, we have a working implementation of the control, but it was only possible by duplicating SubstanceCommandToggleButtonUI then inserting the new UI elements. It would seem to be better engineering to extend the existing class, so we only have to add/maintain new functionality.

If we've missed an approach that better meets the spirit/intention of Radiance, then I welcome your guidance :-)

@kirill-grouchnikov
Copy link
Owner

I think it would be best to provide this functionality in Flamingo itself.

I'll need to check a few things first. The separation of JCommandButton vs JCommandToggleButton always felt a bit weird to me. I think I tried to mirror the separation between JButton and JToggleButton in core Swing, but of course that was a long time ago, so I don't quite remember my line of reasoning.

Now they are both created from a Command object, and there's a lot of similar / identical code between the basic and Substance UI delegates of these two classes. And of course there's JCommandMenuButton and JCommandToggleMenuButton.

Maybe this distinction isn't right any more. Same as there is no separate JCommandSplitButton or JCommandPopupButton - these behavioral aspects are controlled by different content model (command) bits, the same as the toggle-ability.

It would be a breaking change though to combine these four classes into two, along with the matching changes to combine the underlying model implementations - ActionRepeatableButtonModel and ActionToggleButtonModel.

@IanMayo
Copy link
Author

IanMayo commented Jan 9, 2020

provide this functionality in Flamingo itself

Sure, @kirill-grouchnikov - it's your show!

I guess we can keep our "local" version for now, and adopt a Flamingo version when it arrives.

@kirill-grouchnikov
Copy link
Owner

I'm going to change the title of this bug to reflect the more narrow scope of the feature request.

@kirill-grouchnikov kirill-grouchnikov changed the title [Flamingo] Allow Flamingo components to be sub-classed [Flamingo] Evolution of command button component classes Jan 9, 2020
@kirill-grouchnikov kirill-grouchnikov self-assigned this Jan 9, 2020
@kirill-grouchnikov kirill-grouchnikov added 3.0 - Diamond Release 3.0 - 2020.H1 Breaking change Marking changes that break backwards compatibility labels Jan 9, 2020
@IanMayo
Copy link
Author

IanMayo commented Jan 9, 2020

My original intention was that once the class in question became extensible we'd contribute a PR containing the new feature.

But, thanks for generously taking it on yourself, @kirill-grouchnikov .

@kirill-grouchnikov
Copy link
Owner

There are no plans to make the UI delegate classes be part of the public API surface.

kirill-grouchnikov added a commit that referenced this issue Jan 10, 2020
Fold JCommandToggleButton into JCommandButton. Fold JCommandToggleMenuButton into JCommandMenuButton.

Tracked by #231
@kirill-grouchnikov
Copy link
Owner

The first pass is done. See sample in https://github.com/kirill-grouchnikov/radiance/blob/master/demos/flamingo-demo/src/main/java/org/pushingpixels/demo/flamingo/common/TestCommandToggleButtons.java#L116 on how to combine toggle for main action with secondary content.

The second pass (tentative) will fold JCommandMenuButton into JCommandButton

@kirill-grouchnikov
Copy link
Owner

Overall 960 lines fewer in the codebase after consolidating the command button component classes and their UI delegates.

@IanMayo
Copy link
Author

IanMayo commented Jan 14, 2020

Thanks for this support @kirill-grouchnikov We have a temporary fix in place, until the next Flamingo release. To help our planning - do you have a planned date for the release of 3.0 ?

@kirill-grouchnikov
Copy link
Owner

https://github.com/kirill-grouchnikov/radiance/blob/master/versions.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.0 - Diamond Release 3.0 - 2020.H1 Breaking change Marking changes that break backwards compatibility
Projects
None yet
Development

No branches or pull requests

2 participants