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

To be able to extend DisplayModelWaveBean. #153

Closed
Rizen59 opened this Issue Jul 21, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@Rizen59
Contributor

Rizen59 commented Jul 21, 2015

Hi Seb,

We decided to migrate from the very very very old Jrebirth 7.7.0 to 8.0.2 this week, a good refresh for the project :)
So we will probably add some issues/improvements for Jrebirth or send you pull requests.

Here is the first one.
In our project we have extended the DisplayModelWaveBean to add more options.
To get and use them we added more commands to execute before/after ShowModelCommand, DetachModelCommand...

But in Jrebirth 8.0.2 the DisplayModelWaveBean can not be extended anymore (since the constructor has become private).
So we can't use this extended waveBean in ShowModelCommand and others because it can not be casted to DisplayModelWaveBean.

Exemple 1:
In a class extended DisplayModelWaveBean we added a display duration (for temporary notification printed to use), and a Command that detach the model at the end of this duration.

Exemple 2:
In a class extended DisplayModelWaveBean we added some popup properties for model displayed in popup box.

Here are some ideas :

  • Changing the constructor to protected. The developper will have to not forget to add a create method
  • Add an interface to DisplayModelWaveBean. This will allow to use a pattern such as decorator to extends the class possibilies.

It will be interesting to have your opinion on this method. I can provide you an example with Jrebirth 7.7.0 if necessary.

Regards,

@sbordes sbordes added the question label Jul 21, 2015

@sbordes sbordes self-assigned this Jul 21, 2015

@sbordes sbordes added this to the 8.0.3 milestone Jul 21, 2015

@sbordes

This comment has been minimized.

Show comment
Hide comment
@sbordes

sbordes Jul 21, 2015

Member

Gloups, I have closed the api too late, the DMWB should not be specialized because it's used by internal methods (displayUI, that can be triggered from Wave)., but I understand your needs to add cool ergonomic features to this basic behavior.

I don't know yet the best way to add features to internal-commands, perhaps by adding hooks to call other commands between the prepare-show steps.

Meanwhile,
Can you wrap the ShowModelCommand into your own MultiCommand that start a timer and call a RemoveCommand.
Could popup properties be added using WaveData ?

I don't know how deep are your changes, so if you have piece of code it could help

Member

sbordes commented Jul 21, 2015

Gloups, I have closed the api too late, the DMWB should not be specialized because it's used by internal methods (displayUI, that can be triggered from Wave)., but I understand your needs to add cool ergonomic features to this basic behavior.

I don't know yet the best way to add features to internal-commands, perhaps by adding hooks to call other commands between the prepare-show steps.

Meanwhile,
Can you wrap the ShowModelCommand into your own MultiCommand that start a timer and call a RemoveCommand.
Could popup properties be added using WaveData ?

I don't know how deep are your changes, so if you have piece of code it could help

@Rizen59

This comment has been minimized.

Show comment
Hide comment
@Rizen59

Rizen59 Jul 21, 2015

Contributor

Using wave datas can be a solution, I will try this way for now and show you the result.
This issue was more to keep a simple code, as you can see with these pieces of code under jrebirth 7.7.0:

The WaveBean : http://pastebin.com/BWp2LiBs
The extended ShowModelCommand : http://pastebin.com/H5X4axJW
The Command that detach and fade the model (for notification popup) : http://pastebin.com/n8Ejgfrr

Contributor

Rizen59 commented Jul 21, 2015

Using wave datas can be a solution, I will try this way for now and show you the result.
This issue was more to keep a simple code, as you can see with these pieces of code under jrebirth 7.7.0:

The WaveBean : http://pastebin.com/BWp2LiBs
The extended ShowModelCommand : http://pastebin.com/H5X4axJW
The Command that detach and fade the model (for notification popup) : http://pastebin.com/n8Ejgfrr

@Rizen59

This comment has been minimized.

Show comment
Hide comment
@Rizen59

Rizen59 Jul 21, 2015

Contributor

Notice that we have much more properties, that's why a WaveBean was more convenient here.

Contributor

Rizen59 commented Jul 21, 2015

Notice that we have much more properties, that's why a WaveBean was more convenient here.

@sbordes

This comment has been minimized.

Show comment
Hide comment
@sbordes

sbordes Jul 21, 2015

Member

Yes this is their goals (wrapping a lot of properties).

I will update the DMWB constructor visibility to protected, to let customization possible but I will do something to be able to customize the basic behavior by using a specialized DMWB (recopied into dispalyUi internal method)

Thus it will be possible to subclass DMWB to add a lot of properties and command hook will be added into ShowModelCommand.

Member

sbordes commented Jul 21, 2015

Yes this is their goals (wrapping a lot of properties).

I will update the DMWB constructor visibility to protected, to let customization possible but I will do something to be able to customize the basic behavior by using a specialized DMWB (recopied into dispalyUi internal method)

Thus it will be possible to subclass DMWB to add a lot of properties and command hook will be added into ShowModelCommand.

@Rizen59

This comment has been minimized.

Show comment
Hide comment
@Rizen59

Rizen59 Jul 21, 2015

Contributor

That's a great news ! And very quick also :).
Hope I'm not annoying you during your holidays ^^

Contributor

Rizen59 commented Jul 21, 2015

That's a great news ! And very quick also :).
Hope I'm not annoying you during your holidays ^^

@sbordes sbordes modified the milestones: 8.1.0, 8.0.3 Jul 21, 2015

@sbordes

This comment has been minimized.

Show comment
Hide comment
@sbordes

sbordes Jul 21, 2015

Member

constructor visibility set to to protected (8.0.3-SNAPSHOT on OJO)
Hooks will be implemented asap

Member

sbordes commented Jul 21, 2015

constructor visibility set to to protected (8.0.3-SNAPSHOT on OJO)
Hooks will be implemented asap

@sbordes

This comment has been minimized.

Show comment
Hide comment
@sbordes

sbordes Aug 7, 2015

Member

DisplayModelWaveBean 's Constructor has been reset to private because definitely it's not a good thing to specialize it.

Instead, I have refactored the WaveBean code. Now a wave can handle several WaveBean (one per type). Command can still have a dedicated WaveBean (even if code can be slightly improved to detect the kind).

The attachUi method was fixed and can call another command than ShowModelCommand, a sample is provided within MasteringTables application.

// Create your own Wave Bean handled by your own Command FadeInOutCommand
final FadeInOutWaveBean fiowb = FadeInOutWaveBean.create().showDuration(Duration.millis(4000));

 attachUi(AdModel.class, Builders.buildUiData(getView().topPlaceHolder(), fiowb, ShowTemporaryCommand.class).toArray(new WaveData[0]));

The new Grouped Command

public class ShowTemporaryCommand extends DefaultMultiBeanCommand<DisplayModelWaveBean> {
    @Override
    protected List<UniqueKey<? extends Command>> defineSubCommand() {
        return Arrays.asList(
                             getCommandKey(PrepareModelCommand.class),
                             getCommandKey(AttachModelCommand.class),
                             getCommandKey(FadeInOutCommand.class),
                             getCommandKey(DetachModelCommand.class)
                     );
    }
}

And the fading command

public class FadeInOutCommand extends DefaultUIBeanCommand<FadeInOutWaveBean> {
    @Override
    protected void perform(final Wave wave) throws CommandException {

        final Node node = getNode(wave);

        final FadeInOutWaveBean waveBean = getWaveBean(wave);

        final FadeTransition fadeIn = FadeTransitionBuilder.create()
                                                           .autoReverse(false)
                                                           .cycleCount(1)
                                                           .fromValue(0)
                                                           .toValue(1.0)
                                                           .duration(waveBean.fadingInDuration())
                                                           .node(node)
                                                           .build();

        final PauseTransition pause = PauseTransitionBuilder.create().duration(waveBean.showDuration()).build();

        final FadeTransition fadeOut = FadeTransitionBuilder.create()
                                                            .autoReverse(false)
                                                            .cycleCount(1)
                                                            .fromValue(1.0)
                                                            .toValue(0)
                                                            .duration(waveBean.fadingOutDuration())
                                                            .node(node)
                                                            .build();
        final SequentialTransition pt = SequentialTransitionBuilder.create()
                                                                   .children(fadeIn, pause, fadeOut)
                                                                   .build();
        pt.play();
    }

    private Node getNode(final Wave wave) {
        Node node = getWaveBean(wave).node();
        if (node == null && wave.hasWaveBean(DisplayModelWaveBean.class)) {
            node = wave.waveBean(DisplayModelWaveBean.class).showModel().getRootNode();
        }
        return node;
    }
}
Member

sbordes commented Aug 7, 2015

DisplayModelWaveBean 's Constructor has been reset to private because definitely it's not a good thing to specialize it.

Instead, I have refactored the WaveBean code. Now a wave can handle several WaveBean (one per type). Command can still have a dedicated WaveBean (even if code can be slightly improved to detect the kind).

The attachUi method was fixed and can call another command than ShowModelCommand, a sample is provided within MasteringTables application.

// Create your own Wave Bean handled by your own Command FadeInOutCommand
final FadeInOutWaveBean fiowb = FadeInOutWaveBean.create().showDuration(Duration.millis(4000));

 attachUi(AdModel.class, Builders.buildUiData(getView().topPlaceHolder(), fiowb, ShowTemporaryCommand.class).toArray(new WaveData[0]));

The new Grouped Command

public class ShowTemporaryCommand extends DefaultMultiBeanCommand<DisplayModelWaveBean> {
    @Override
    protected List<UniqueKey<? extends Command>> defineSubCommand() {
        return Arrays.asList(
                             getCommandKey(PrepareModelCommand.class),
                             getCommandKey(AttachModelCommand.class),
                             getCommandKey(FadeInOutCommand.class),
                             getCommandKey(DetachModelCommand.class)
                     );
    }
}

And the fading command

public class FadeInOutCommand extends DefaultUIBeanCommand<FadeInOutWaveBean> {
    @Override
    protected void perform(final Wave wave) throws CommandException {

        final Node node = getNode(wave);

        final FadeInOutWaveBean waveBean = getWaveBean(wave);

        final FadeTransition fadeIn = FadeTransitionBuilder.create()
                                                           .autoReverse(false)
                                                           .cycleCount(1)
                                                           .fromValue(0)
                                                           .toValue(1.0)
                                                           .duration(waveBean.fadingInDuration())
                                                           .node(node)
                                                           .build();

        final PauseTransition pause = PauseTransitionBuilder.create().duration(waveBean.showDuration()).build();

        final FadeTransition fadeOut = FadeTransitionBuilder.create()
                                                            .autoReverse(false)
                                                            .cycleCount(1)
                                                            .fromValue(1.0)
                                                            .toValue(0)
                                                            .duration(waveBean.fadingOutDuration())
                                                            .node(node)
                                                            .build();
        final SequentialTransition pt = SequentialTransitionBuilder.create()
                                                                   .children(fadeIn, pause, fadeOut)
                                                                   .build();
        pt.play();
    }

    private Node getNode(final Wave wave) {
        Node node = getWaveBean(wave).node();
        if (node == null && wave.hasWaveBean(DisplayModelWaveBean.class)) {
            node = wave.waveBean(DisplayModelWaveBean.class).showModel().getRootNode();
        }
        return node;
    }
}

@sbordes sbordes closed this Aug 7, 2015

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