Fixed bug dialog mouse up event

[LittleGnome]

Fixed bug for the dialog, where a mouse up event closes the dialog and results in unexpected UX behavior

Update and fixed README.md;

[LittleGnome]

@EisenbergEffect I don't know why the tests are failing, could you look into this.

Looks like some tests are very unstable.

[EisenbergEffect]

@LittleGnome Unfortunately, there are some test stability issues. Don't worry too much about that. @StrahilKazlachev Are you available to do a review of this?

[LittleGnome]

@StrahilKazlachev I agree that the bug classification is to hard, but the user experience of the dialog is broken.

If a user is selecting a text in a dialog and accidentally hovers with his mouse on the dialog overlay the dialog closes. This can't be explained to a user as designed behaviour. At least I am really curious why this should be breaking?

[StrahilKazlachev]

• accidentally click outside the dialog, if you move the mouse inside and then release, it does not close - currently. Will not be the case with this change.
• integration with 3rd party libs, the results can be unexpected. I'll use the keyEvent setting as an example for such scenario.

I just look over the code, mention any (potential) breaking changes I see, etc. I'll not be making the decision whether or not to merge this one.

[LittleGnome]

@StrahilKazlachev Hmm, thanks voor the explanation! Shall I update this pull request so this mouseEvent is configurable too?

[StrahilKazlachev]

If you want to make such change you'll need to:

• add the setting to the DialogSettings interface
• add the setting to the DefaultDialogSettings class with the current value(click) as default
• update the renderers implementations to use this new setting, there shouldn't be any other place that uses this setting, at least in this repo - excluding tests.
• update the tests to use the setting instead of hardcoded event name.

[LittleGnome]

@StrahilKazlachev are you sure you want this setting in the DefaultDialogSettings? The KeyEvent isn't either present in the DefaultDialogSettings. For now, I have used a fallback to 'click' in the setup and clear clickHandler methods.

[StrahilKazlachev]

The new setting seems OK, it won't be a breaking change now. No intention to go nitpicking, if someone else wants to ...

[dagtveit]

I realy need this update. the user experience due to this bug is very bad. if the user marks text inside a field and drag from right to left and mouse up happens outside the dialog. it gets removed.

[EisenbergEffect]

@bigopon or @StrahilKazlachev can you do a final review on this and make a recommendation on whether we can merge and release.

[StrahilKazlachev]

@EisenbergEffect did agree 1 comment above. Since I'm pretty much inactive, I decided it is inappropriate to merge without some else also going over it.

[EisenbergEffect]

Let's have @bigopon jump in and do a final review so it's fresh and up to date based on his knowledge. If he doesn't have any issues, then we'll get it merged and released.

[bigopon]

@LittleGnome @dagtveit sorry for the late review. Will watch for this. I left 2 comments, besides those, I think it's good

[LittleGnome]

@dagtveit Will update this pull request with the rework of the code reviews.

[StrahilKazlachev]

@dagtveit PRs are not the right place to ask such questions, especially in the review comments. Also doesn't overlayDismiss: false do exactly what you want? Can also combine with keyboard: false if only pressing the buttons is desired.

[dagtveit]

@dagtveit PRs are not the right place to ask such questions, especially in the review comments. Also doesn't overlayDismiss: false do exactly what you want? Can also combine with keyboard: false if only pressing the buttons is desired.

srry for missposting Hey that was a desired workaround to the original issue. and i completly missed that option. did work perfectly. but also looking forward to the bug fix. i deleted my original comment.

[LittleGnome]

@EisenbergEffect @bigopon Do I need to bump the version number? Because the build_test step fails on a not bumped version number.

[EisenbergEffect]

@LittleGnome No need to mess with the version number or anything in the dist folder. I'll handle all that when we do the release. Let us know when you are ready for a final review and we'll make it happen.

Thank you everyone for your patience!

[LittleGnome]

@EisenbergEffect Okay, then it's ready for the final review!

[LittleGnome]

@bigopon can you do a final review?

[EisenbergEffect]

Thank you @bigopon We'll work to get this out today or tomorrow. There's a bit of "unknown" in this week's schedule, so it may take a bit longer, but hopefully not. Thank you everyone for your patience.