qt: Enable tabbing through labels #14810

pull hebasto wants to merge 3 commits into bitcoin:master from hebasto:20181124-tab-through-labels changing 8 files +161 −180
  1. hebasto commented at 4:21 pm on November 26, 2018: member

    This PR:

    • fixes a visual bug described in #14577
    • improves UX by enabling moving focus through fields using keyboard (Tab and Shift+Tab as usual) on Overview tab of the main window and on Info tab of the Debug window (other tabs leaved untouched as tab order for them seems a bit entangled). Inspired by laanwj’s #14577 (comment)

    Also applied a style-fix from #15220:

    please just do this the next time you’re editing these files.

  2. laanwj commented at 6:09 pm on November 26, 2018: member

    Concept ACK, thanks!

    On Mon, Nov 26, 2018, 17:21 Hennadii Stepanov <notifications@github.com wrote:

    Inspired by @laanwj https://github.com/laanwj’s #14577 (comment) https://github.com/bitcoin/bitcoin/pull/14577#issuecomment-438652059

    This PR improves UX by enabling moving focus through QLabel fields using keyboard (Tab and Shift+Tab as usual).

    On the way this PR fixes a visual bug described in #14577 https://github.com/bitcoin/bitcoin/pull/14577 and improves UX for help windows.

    cc: @promag https://github.com/promag @fanquake https://github.com/fanquake @jonasschnelli https://github.com/jonasschnelli

    You can view, comment on, or merge this pull request online at:

    #14810 Commit Summary

    • Add event filter for focused QLabel objects
    • Enable tabbing through labels for Debug Window
    • Enable tabbing through labels for Overview view
    • Improve UX for help dialogs
    • Enable tabbing through labels for Send view
    • Enable tabbing through labels for Coin Selection

    File Changes

    Patch Links:

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/14810, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHutqZNtoB4ikOhZ9AnLhCNU6b3U0-Yks5uzBUegaJpZM4YzXb1 .

  3. DrahtBot commented at 7:25 pm on November 26, 2018: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #19193 (qt: Deduplicate NumConnections enum by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  4. fanquake added the label GUI on Nov 26, 2018
  5. jonasschnelli commented at 8:40 pm on December 9, 2018: contributor
    The only downside with this is, that you can’t select parts of the text of a label.
  6. laanwj commented at 12:43 pm on December 13, 2018: member
    @jonasschnelli Yeah. Possibly we’re getting too deep in shed-painting territory here. I’ll leave the decision what to do here up to you.
  7. jonasschnelli commented at 7:29 pm on December 13, 2018: contributor
    More opinions, tests would be welcome.
  8. hebasto commented at 10:14 am on December 15, 2018: member

    @jonasschnelli @laanwj

    The only downside with this is, that you can’t select parts of the text of a label.

    That has been made intentionally. Rationale: some labels are modified by setText() and any user selected text will vanish. In such circumstances, only two options are consistent: all-selected or none-selected. To make a focused label clearly visible to a user the former option has been chosen.

  9. DrahtBot added the label Needs rebase on Jan 16, 2019
  10. hebasto force-pushed on Jan 20, 2019
  11. DrahtBot removed the label Needs rebase on Jan 20, 2019
  12. hebasto force-pushed on Jan 21, 2019
  13. hebasto commented at 0:40 am on January 21, 2019: member
    Rebased.
  14. hebasto commented at 0:42 am on January 21, 2019: member
    @promag @Sjors would you mind reviewing this PR?
  15. promag commented at 5:06 pm on January 21, 2019: member
    @hebasto sure, on my list now.
  16. Sjors commented at 6:32 pm on January 21, 2019: member

    Concept ACK.

    Unfortunately this is a bit difficult to fully test on macOS because the menu keyboard shortcuts don’t work (#13832), so you need the mouse for most navigation anyway.

    I’m confused why forms/overviewpage.ui is modified; what’s the point of being able to focus on the amounts? Is it so they’re easier to copy-paste? Or is it an accessibility feature?

    I was already able to navigate through the peer list. Now with this change I can select and copy individual items.

    When I tab through the Send screen, it only goes through Pay to, label, amount, [nothing] and then the balance field.

    On the receive screen it skips the BTC dropdown field, the payment request button and the erase button. I can go through the list of historical payment request, but I can’t access their view and delete buttons.

    On the transactions page I can’t access the the three filter dropdowns (on the left), and also not the Export button.

    I also can’t reach the wallet switcher.

  17. hebasto commented at 7:51 pm on January 21, 2019: member

    @Sjors Thank you for your review and valuable input.

    I’m confused why forms/overviewpage.ui is modified; what’s the point of being able to focus on the amounts? Is it so they’re easier to copy-paste? Or is it an accessibility feature?

    Yes, it is an accessibility feature mostly. Ref: #14577 (comment).

    All others points will be addressed soon.

  18. Sjors commented at 2:53 pm on January 22, 2019: member
    In that case, is there a way to trigger the tooltip by keyboard too?
  19. promag commented at 1:31 pm on April 29, 2019: member
    Just wondering if changing to QLineEdit with readOnly=true makes sense? At least visually it’s clear?
  20. promag commented at 1:37 pm on April 29, 2019: member
    BTW I’ve noticed that #12616 no longer works.
  21. in src/qt/guiutil.h:159 in ba5fc320c7 outdated 
    154+    class FocusedLabelFilter : public QObject
155+    {
156+        Q_OBJECT
157+
158+    public:
159+        explicit FocusedLabelFilter(QObject* parent = 0);
    practicalswift commented at 4:15 pm on May 7, 2019:
    Use nullptr instead of 0 :-)
  22. jonasschnelli commented at 9:15 am on October 9, 2019: contributor
    Tested this again and the lack of ability to select parts of an amount label (say the amount without the unit BTC) makes me NACK this as it is now.
  23. hebasto closed this on Oct 9, 2019
  24. hebasto reopened this on Jan 9, 2020
  25. hebasto force-pushed on Jan 9, 2020
  26. hebasto commented at 11:28 am on January 9, 2020: member

    @promag

    Just wondering if changing to QLineEdit with readOnly=true makes sense? At least visually it’s clear?

    This approach does not work for me (background color issues), see #17898.

    BTW I’ve noticed that #12616 no longer works.

    Fixed in the latest push.

    @jonasschnelli

    The only downside with this is, that you can’t select parts of the text of a label.

    Tested this again and the lack of ability to select parts of an amount label (say the amount without the unit BTC) makes me NACK this as it is now.

    Fixed in the latest push.

  27. promag commented at 11:33 am on January 9, 2020: member
    @hebasto thanks for trying my suggestion in #17898. Will try and review both.
  28. hebasto commented at 11:38 am on January 9, 2020: member
    The OP has been updated.
  29. hebasto commented at 11:41 am on January 9, 2020: member

    @Sjors

    In that case, is there a way to trigger the tooltip by keyboard too?

    It seems possible. This could be addresses in a follow up PR ;)

  30. hebasto force-pushed on Jan 9, 2020
  31. hebasto commented at 12:21 pm on January 9, 2020: member
    Invariants are forced by HighlightLabelWidget ctor.
  32. hebasto commented at 4:00 pm on April 11, 2020: member
    @jonasschnelli Mind re-reviewing?
  33. qt: Add HighlightLabelWidget 7150d90538
  34. qt: Make Information tab accessible via keyboard 191187ca0c
  35. qt: Make Overview tab accessible via keyboard 1bd1e604ae
  36. hebasto force-pushed on Apr 11, 2020
  37. hebasto commented at 4:10 pm on April 11, 2020: member
    Rebased d356340b455903ebc58a29da06b481f1e5c9775e -> 1bd1e604aebc0e1dd437cd294195332080596223 (pr14810.101 -> pr14810.102) due to the conflict with #18402.
  38. jonasschnelli commented at 7:14 am on May 29, 2020: contributor

    I’m unsure if it is worth customising the label based on mouse events to achieve tabbing thought labels. Also, on macOS, the selected color is different and almost not seeable.

    master:

    this PR:

  39. jonasschnelli added the label Waiting for author on May 29, 2020
  40. hebasto commented at 2:03 pm on June 8, 2020: member

    This PR:

    • fixes a visual bug described in #14577

    This fix is replaced by #19210.

    • improves UX by enabling moving focus through fields using keyboard (Tab and Shift+Tab as usual) on Overview tab of the main window and on Info tab of the Debug window (other tabs leaved untouched as tab order for them seems a bit entangled). Inspired by laanwj’s #14577 (comment) @jonasschnelli

    I’m unsure if it is worth customising the label based on mouse events to achieve tabbing thought labels. Also, on macOS, the selected color is different and almost not seeable.

    I have some ideas how to get the keyboard-only UX but not going to work on it right now. So closing this PR.

    Please label it “Up for grabs” :)

  41. hebasto closed this on Jun 8, 2020
  42. fanquake removed the label Waiting for author on Jul 9, 2020
  43. laanwj referenced this in commit d626a3be31 on Jul 15, 2020
  44. hebasto deleted the branch on Jul 15, 2020
  45. sidhujag referenced this in commit 6db274eaa0 on Jul 16, 2020
  46. PastaPastaPasta referenced this in commit 3b0144beeb on Sep 21, 2021
  47. kittywhiskers referenced this in commit f8470b479a on Oct 12, 2021
  48. DrahtBot locked this on Feb 15, 2022


hebasto laanwj DrahtBot jonasschnelli promag Sjors practicalswift

Labels
GUI

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 15:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me