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

fix: [#1431] Dispatch the hidePlayButton on the Button Event to prevent that keep on the screen on some situations [#1431] #2066

Conversation

catrielmuller
Copy link
Contributor

@catrielmuller catrielmuller commented Oct 23, 2021

Hi @kamranayub , I'm tried to reproduce the issue [#1431] that was resolved on the commit ( c5d710a )
So I created a Story to test this on different situations, I notices that if you use something like this

heart.on('pointerdown', async (_evnt: ex.Input.PointerEvent) => {
      if (!game.isPaused()) {
        game.stop();
        await loader.showPlayButton();
        game.start();
      }
    });

to implement a play/pause functionality, the showPlayButton show the button, however you need hide manually after the user click them again, to prevent that I recomend move that logic inside of the ShowPlayButton click event definition.

Also I added a Options property to the withEngine util to override the default engine options used by the all Stories.

===:clipboard: PR Checklist :clipboard:===

  • 📌 issue exists in github for these changes
  • 🔬 existing tests still pass
  • 🙈 code conforms to the style guide
  • 📐 new tests written and passing / old tests updated with new scenario(s)
  • 📄 changelog entry added (or not needed)

==================

Closes #1431

Changes:

  • withEngine utils support an aditional options parameter to override the Engine default options.
  • Story to show a play / pause implementation.
  • Dispatch the hidePlayButton on the Button Event to prevent that keep on the screen on some situations [Play button is not cleaned up on stop #1431].

@catrielmuller catrielmuller force-pushed the catriel/play-button-cleaned-up-on-stop branch from e64cebb to 46b8653 Compare October 23, 2021 06:31
@eonarheim eonarheim changed the title Dispatch the hidePlayButton on the Button Event to prevent that keep on the screen on some situations [#1431] fix: [#1431] Dispatch the hidePlayButton on the Button Event to prevent that keep on the screen on some situations [#1431] Oct 23, 2021
@@ -255,6 +256,8 @@ export class Loader extends Class implements Loadable<Loadable<any>[]> {
const startButtonHandler = (e: Event) => {
// We want to stop propogation to keep bubbling to the engine pointer handlers
e.stopPropagation();
// Hide Button after click
this.hidePlayButton();
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -47,7 +48,8 @@ export const withEngine = (storyFn: (game: Engine, args?: Record<string, any>) =
canvasElement: canvas,
displayMode: DisplayMode.FitScreen,
suppressPlayButton: true,
pointerScope: Input.PointerScope.Canvas
pointerScope: Input.PointerScope.Canvas,
...options
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@eonarheim eonarheim left a comment

Choose a reason for hiding this comment

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

@catrielmuller Thanks again! I like pause/start example in the story, definitely something we might port into documentation around pause/play

@eonarheim eonarheim merged commit aa499c8 into excaliburjs:main Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Play button is not cleaned up on stop
2 participants