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

Terminal eats tab navigation shortcuts #127

Closed
jpnurmi opened this issue Oct 10, 2022 · 7 comments · Fixed by #133
Closed

Terminal eats tab navigation shortcuts #127

jpnurmi opened this issue Oct 10, 2022 · 7 comments · Fixed by #133
Labels
bug Something isn't working

Comments

@jpnurmi
Copy link
Contributor

jpnurmi commented Oct 10, 2022

Ctrl+PageDn and Ctrl+PageUp are supposed to switch to the next or previous tab, respectively. These no longer work if there's an open terminal in any tab but only if all are showing the home page.

Screencast.from.2022-10-10.08-19-30.webm

Ref: #108

@jpnurmi jpnurmi added the bug Something isn't working label Oct 10, 2022
@jpnurmi
Copy link
Contributor Author

jpnurmi commented Oct 10, 2022

xterm v2 works

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:xterm/flutter.dart';
import 'package:xterm/xterm.dart';

void main() {
  runApp(const MaterialApp(home: MyHomePage()));
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key});

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  final _terminal = Terminal(maxLines: 10000);

  @override
  Widget build(BuildContext context) {
    return CallbackShortcuts(
      bindings: {
        const SingleActivator(LogicalKeyboardKey.pageDown): () {
          print('page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp): () {
          print('page up');
        },
        const SingleActivator(LogicalKeyboardKey.pageDown, control: true): () {
          print('ctrl+page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp, control: true): () {
          print('ctrl+page up');
        },
        const SingleActivator(LogicalKeyboardKey.pageDown, shift: true): () {
          print('shift+page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp, shift: true): () {
          print('shift+page up');
        },
        const SingleActivator(LogicalKeyboardKey.pageDown,
            control: true, shift: true): () {
          print('ctrl+shift+page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp,
            control: true, shift: true): () {
          print('ctrl+shift+page up');
        },
        const SingleActivator(LogicalKeyboardKey.keyT,
            control: true, shift: true): () {
          print('ctrl+shift+T');
        },
      },
      child: Focus(
        autofocus: true,
        child: Scaffold(
          appBar: AppBar(title: const Text('xterm')),
          body: TerminalView(
            terminal: _terminal,
            autofocus: true,
          ),
        ),
      ),
    );
  }
}

xterm v3 doesn't work

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:xterm/xterm.dart';

void main() {
  runApp(const MaterialApp(home: MyHomePage()));
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key});

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  final _terminal = Terminal();

  @override
  Widget build(BuildContext context) {
    return CallbackShortcuts(
      bindings: {
        const SingleActivator(LogicalKeyboardKey.pageDown): () {
          print('page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp): () {
          print('page up');
        },
        const SingleActivator(LogicalKeyboardKey.pageDown, control: true): () {
          print('ctrl+page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp, control: true): () {
          print('ctrl+page up');
        },
        const SingleActivator(LogicalKeyboardKey.pageDown, shift: true): () {
          print('shift+page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp, shift: true): () {
          print('shift+page up');
        },
        const SingleActivator(LogicalKeyboardKey.pageDown,
            control: true, shift: true): () {
          print('ctrl+shift+page down');
        },
        const SingleActivator(LogicalKeyboardKey.pageUp,
            control: true, shift: true): () {
          print('ctrl+shift+page up');
        },
        const SingleActivator(LogicalKeyboardKey.keyT,
            control: true, shift: true): () {
          print('ctrl+shift+T');
        },
      },
      child: Focus(
        autofocus: true,
        child: Scaffold(
          appBar: AppBar(title: const Text('xterm')),
          body: TerminalView(
            autofocus: true,
            _terminal,
          ),
        ),
      ),
    );
  }
}

@xtyxtyx
Copy link

xtyxtyx commented Oct 10, 2022

v2 internally uses RawKeyboardListener which fire keyboard events no matter whether the event is consumed by the focus system or not. In v3, TerminalView uses Focus to listen for keyboard events, therefore events consumed by the terminal stop propagating to upper FocusNodes in the focus tree.

For now it's recommended to use TerminalView.shortcuts to make shortcuts have higher priority than the input handler of the terminal.

@jpnurmi
Copy link
Contributor Author

jpnurmi commented Oct 10, 2022

Hi @xtyxtyx, thanks for chiming in. TerminalStudio/xterm.dart#134 also helps with this because it makes it possible to pair the map of shortcuts with actions. The above example is using callback shortcuts for the sake of simplicity but this is not the case for all shortcuts. :)

@jpnurmi
Copy link
Contributor Author

jpnurmi commented Oct 10, 2022

In v3, TerminalView uses Focus to listen for keyboard events, therefore events consumed by the terminal stop propagating to upper FocusNodes in the focus tree.

Hmm, does Flutter not have a system to let shortcuts handle key events before normal key event handlers are called? It seems problematic that a normal key handler deep down the tree blocks the entire app's shortcuts.

@xtyxtyx
Copy link

xtyxtyx commented Oct 10, 2022

TerminalStudio/xterm.dart#134 also helps with this because it makes it possible to pair the map of shortcuts with actions.

Sorry for the delay in responding to the PR. I'm trying to figure out a better design of the interface to add shortcuts to the terminal. Will merge the PR as soon as possible once the design is finalized.

@xtyxtyx
Copy link

xtyxtyx commented Oct 10, 2022

Hmm, does Flutter not have a system to let shortcuts handle key events before normal key event handlers are called?

No way as far as I know... Perhaps the terminal is consuming too much key bindings by default. I'm planning to deprecate the keytab and instead turn it into something like List<TerminalKeybinding> to make it easier to customize the default key bindings.

jpnurmi added a commit to jpnurmi/xterm.dart that referenced this issue Oct 12, 2022
jpnurmi added a commit to jpnurmi/workshops that referenced this issue Oct 12, 2022
Includes a workaround for tab navigation shortcuts:
jpnurmi/xterm.dart@3655c3d

Close: canonical#127
jpnurmi added a commit that referenced this issue Oct 12, 2022
Includes a workaround for tab navigation shortcuts:
jpnurmi/xterm.dart@3655c3d

Close: #127
@jpnurmi
Copy link
Contributor Author

jpnurmi commented Oct 13, 2022

@xtyxtyx What about something similar to what Flutter does with text editing shortcuts? Something you put high up in the tree to be able to override them below.

https://api.flutter.dev/flutter/widgets/DefaultTextEditingShortcuts-class.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants