Skip to content

fix UX issues in move debugger#640

Merged
delucis merged 4 commits into
masterfrom
ux
May 2, 2020
Merged

fix UX issues in move debugger#640
delucis merged 4 commits into
masterfrom
ux

Conversation

@nicolodavis
Copy link
Copy Markdown
Member

@nicolodavis nicolodavis commented Apr 23, 2020

Checklist

  • Use a separate branch in your local repo (not master).
  • Test coverage is 100% (or you have a story for why it's ok).

Fixes #618

@nicolodavis nicolodavis requested a review from delucis April 23, 2020 02:42
Comment thread src/client/debug/main/InteractiveFunction.svelte
@delucis
Copy link
Copy Markdown
Member

delucis commented Apr 23, 2020

This is an additional change, but given it’s in the same scope:

When the debug panel is hidden using the . shortcut, the hotkeys for toggling which panel is displayed (m, l, i & a) are still active. This is a minimal bug, because the controls within the panels seem to be disabled correctly, but might be a simple fix to include here to ensure the panels aren’t toggled while hidden.

@delucis
Copy link
Copy Markdown
Member

delucis commented Apr 23, 2020

Did some local testing and this looks good for disabling shortcuts inside the move content editables 👍

To fix the copy/paste issue, I think we can add checks here:

https://github.com/nicolodavis/boardgame.io/blob/06122c7e5b4e0f2f0cfe13ce8bb899c6e37903ad/src/client/debug/main/Hotkey.svelte#L26

Something like:

    if (
      !$disableHotkeys && !disable &&
      !e.ctrlKey && !e.metaKey &&
      e.key == value
    ) { 

I don’t think we need to check for alt or shift keys as they tend to produce different e.key values?

To fix toggling the displayed pane while the debug panel is hidden, I think we need to move this code inside an if statement that checks if visible is true:

https://github.com/nicolodavis/boardgame.io/blob/06122c7e5b4e0f2f0cfe13ce8bb899c6e37903ad/src/client/debug/Debug.svelte#L37-L41

@delucis delucis merged commit 1483a08 into master May 2, 2020
@delucis delucis deleted the ux branch May 2, 2020 19:06
@nicolodavis
Copy link
Copy Markdown
Member Author

Thanks @delucis!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UX issues with contenteditable editors in the debugger

2 participants