Refactor dialog processing #78
Conversation
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
I think this is ready to check now. Maybe the next refactor could merge the remaining two dialog functions, so you can pass the button label and callback for when it's clicked. I think it would make the code much more readable and less complex logic. I can do that over the weekend. |
|
@KevinBatdorf awesome. Good job. |
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
|
🙏 Thanks for your pull request @KevinBatdorf, The team will now review and merge this request. In the mean time why not check out some of the other opensource projects available, contributions are greatly appreciated! Some of the most popular are PR Statistics
|
Done. I think combining the options container would be a next step as well in a followup PR, and having one place for all dialogs. |
|
@KevinBatdorf that is a good idea. |
I couldn't reproduce the bug in #76 but I refactored the code and removed some of the unnecessary logic. I'm hoping this addresses it.
I merged in the two character/no character functions and named it
displayDialogue. This makes it easier to manage the next/close buttons and the name can easily be checked before prepending it to the string. It also narrows the surface for bugs.Some new rules I introduced:
Can you test it out? I can make changes tomorrow if needed (or in a follow up PR if its good enough to merge)
closes #76