Skip to content

qt: Cleanup textInteractionFlags for QLabel #14577

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.

Sign up for GitHub

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

Jump to bottom
Closed
hebasto wants to merge 1 commit into from
Closed

qt: Cleanup textInteractionFlags for QLabel #14577

hebasto wants to merge 1 commit into from

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Oct 25, 2018

Qt::LinksAccessibleByMouse removed for all label not containing links.

Qt::TextSelectableByKeyboard removed for all labels due to showing a text cursor even after leaving the label (for all platforms):
screenshot from 2018-10-26 00-54-16

@hebasto
Cleanup textInteractionFlags for QLabel
e4f00c2 
Qt::TextSelectableByMouse - keep unchanged.
Qt::TextSelectableByKeyboard - removed due to showing a text cursor even
after leaving the label.
Qt::LinksAccessibleByMouse - used only by `helpmessagedialog.ui`.
@fanquake fanquake added the GUI label Oct 25, 2018
@promag
Copy link
Contributor

promag commented Oct 26, 2018 •
edited
Loading

utACK e4f00c2.

showing a text cursor even after leaving the label

Nice, tested this on macOS.

@fanquake
Copy link
Member

fanquake commented Oct 28, 2018

tACK e4f00c2

There's one usage of LinksAccessibleByMouse left in helpmessagedialog.ui, because the help message dialog is used to display the About screen, which contains clickable links.

The only other flag we use (including TextSelectableByMouse) is a single instance of NoTextInteraction in debugwindow.ui.

For reference all Qt::TextInteractionFlags are listed here.

Only other thought is if this has some sort of effect to Accessibility, however I can't test that.

Before:
before

After:
after

@laanwj
Copy link
Member

laanwj commented Nov 1, 2018 •
edited
Loading

I'm not sure this is a good idea—why disallow selecting by keyboard?

Only other thought is if this has some sort of effect to Accessibility, however I can't test that.

Exactly, might be that someone is not able to use a mouse that well, being able to tab through fields and select by keyboard could be a great help?

@hebasto
Copy link
Member Author

hebasto commented Nov 1, 2018

@laanwj

Exactly, might be that someone is not able to use a mouse that well, being able to tab through fields and select by keyboard could be a great help?

In fact, someone is not able to tab through fields on master. Because those fields have not Qt::TabFocus flag set.

Therefore, IMO, this PR does not touch accessibility in any way, cleans up code and removes visual artifacts in GUI.

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Nov 13, 2018

Tested ACK.
I think selecting by keyboard would be okay/desirable if you can access the fields by keyboard (which is not possible now).
Bringing it back with full accessibility support (or call it "keyboard" mode) is still possible.

@laanwj
Copy link
Member

laanwj commented Nov 14, 2018

In fact, someone is not able to tab through fields on master. Because those fields have not Qt::TabFocus flag set.

What about enabling that, then? Move in the right direction instead of the wrong one.

@hebasto hebasto mentioned this pull request Nov 26, 2018
Closed
@hebasto
Copy link
Member Author

hebasto commented Nov 26, 2018

@laanwj Thank you for your review.
Closed in favour of #14810.

@hebasto hebasto closed this Nov 26, 2018
@hebasto hebasto deleted the 20181025-textInteractionFlags branch January 18, 2020 10:29
@hebasto hebasto mentioned this pull request Jun 8, 2020
Merged
laanwj added a commit that referenced this pull request Jul 15, 2020
@laanwj
Merge #19210: qt: Get rid of cursor in out-of-focus labels
d626a3b 
bd315eb qt: Get rid of cursor in out-of-focus labels (Hennadii Stepanov)

Pull request description:

  After clicking on `QLabel` with selectable text the cursor remains forever:

  ![47532924-65e7b200-d8ba-11e8-9254-7bde658961cb](https://user-images.githubusercontent.com/32963518/84038485-ad945200-a9a8-11ea-89e3-c7c17d02a611.png)

  This PR fixes this visual bug.

  Earlier attempts to fix this issue:
  - #14577
  - #14810 (combined with other UX feature)

ACKs for top commit:
  promag:
    Code review ACK bd315eb.
  laanwj:
    Tested ACK bd315eb

Tree-SHA512: 6bf89362412e5ce9a4dec6944b62fe44fc31ca49cda7f6e2eb37e847fac9dccb68bca7ac6877b19e42add2333e40d0b4265757ead105ac0a5d28f8ab43b322c3
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 16, 2020
@laanwj
Merge bitcoin#19210: qt: Get rid of cursor in out-of-focus labels
6db274e 
bd315eb qt: Get rid of cursor in out-of-focus labels (Hennadii Stepanov)

Pull request description:

  After clicking on `QLabel` with selectable text the cursor remains forever:

  ![47532924-65e7b200-d8ba-11e8-9254-7bde658961cb](https://user-images.githubusercontent.com/32963518/84038485-ad945200-a9a8-11ea-89e3-c7c17d02a611.png)

  This PR fixes this visual bug.

  Earlier attempts to fix this issue:
  - bitcoin#14577
  - bitcoin#14810 (combined with other UX feature)

ACKs for top commit:
  promag:
    Code review ACK bd315eb.
  laanwj:
    Tested ACK bd315eb

Tree-SHA512: 6bf89362412e5ce9a4dec6944b62fe44fc31ca49cda7f6e2eb37e847fac9dccb68bca7ac6877b19e42add2333e40d0b4265757ead105ac0a5d28f8ab43b322c3
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 21, 2021
@laanwj @PastaPastaPasta
Merge bitcoin#19210: qt: Get rid of cursor in out-of-focus labels
3b0144b 
bd315eb qt: Get rid of cursor in out-of-focus labels (Hennadii Stepanov)

Pull request description:

  After clicking on `QLabel` with selectable text the cursor remains forever:

  ![47532924-65e7b200-d8ba-11e8-9254-7bde658961cb](https://user-images.githubusercontent.com/32963518/84038485-ad945200-a9a8-11ea-89e3-c7c17d02a611.png)

  This PR fixes this visual bug.

  Earlier attempts to fix this issue:
  - bitcoin#14577
  - bitcoin#14810 (combined with other UX feature)

ACKs for top commit:
  promag:
    Code review ACK bd315eb.
  laanwj:
    Tested ACK bd315eb

Tree-SHA512: 6bf89362412e5ce9a4dec6944b62fe44fc31ca49cda7f6e2eb37e847fac9dccb68bca7ac6877b19e42add2333e40d0b4265757ead105ac0a5d28f8ab43b322c3
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

GUI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@hebasto @promag @fanquake @laanwj @jonasschnelli