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

Add accessibility keys to Journal #4891 #667

Conversation

iamutkarshtiwari
Copy link
Contributor

This PR add this feature request -> https://bugs.sugarlabs.org/ticket/4891

Now the journal can be accessed via keyboard. Following functions are implemeted ->

  1. Entries can be scrolled with 'Up'/'Down' arrow keys.
  2. Activity can be started/resumed by pressing 'Enter' key.
  3. Detail View can be opened with 'Right' arrow key.
  4. Switching b/w search entry and tree view can done with 'Tab' key.
  5. Escape key can be use to switch back to list from detail view.
  6. Activity can be renamed by typing anything.

Watch the below gif closely(Mouse is not used). 👯
desktop-animation2

@icarito
Copy link
Contributor

icarito commented Mar 13, 2016

Super! Have not tested it but it may be the nicest feature since 0.94.

Thanks Utkarsh for making it happen and Quozl for asking for it
(effectively)!
:-D

El 13/03/16 a las 13:29, Utkarsh Tiwari escibió:

This PR add this feature request -> https://bugs.sugarlabs.org/ticket/4891

Now the journal can be accessed via keyboard. Following functions are
implemeted ->

  1. Entries can be scrolled with |'Up'/'Down'| arrow keys.
  2. Activity can be started/resumed by pressing |'Enter'| key.
  3. |Detail View| can be opened with |'Right'| arrow key.
  4. Switching b/w |search entry| and |tree view| can done with |'Tab'| key.
  5. |Escape| key can be use to switch back to list from detail view.
  6. Activity can be |renamed| by typing anything.

Watch the below gif closely(Mouse is not used). 👯
desktop-animation2
https://cloud.githubusercontent.com/assets/6258810/13730494/05bf6196-e977-11e5-8443-9c6ddcfb640e.gif


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

#667

    Commit Summary


Reply to this email directly or view it on GitHub
#667.

@iamutkarshtiwari
Copy link
Contributor Author

@icarito @samdroid-apps Thanks! If the community likes it, I can work more on it to implement more accessibility features to it. :)

@iamutkarshtiwari
Copy link
Contributor Author

@walterbender @quozl What are your suggestions on this feature?

@samdroid-apps
Copy link
Contributor

Ok, this is amazing. A few comments:

  • When I type the letters to rename, it seems to loose the 1st charecter. It seems to go like
    1. I press 1st letter
    2. Enters textbox, averything selected
    3. I press second letter
    4. I have to go back and enter the 1st letter again :(
  • When you test if the keypress is a letter, you only consider latin charecters. Maybe just make it an else case?
  • Maybe the left arrow key would be good to use for going back to the list view from the details (in addition to the escape key)? It is very intuitive (forward to go in, backward to go out)
  • Maybe a sugar-artwork patch could add a focus indicator to the search bar - so that I know when it is selected?
  • The palettes used in the journal are GtkMenu based ones, which have great keyboard support. Maybe you could add a keyboard shortcut to show the palette? This would allow for keyboard based deletion, copying, etc.

@iamutkarshtiwari
Copy link
Contributor Author

@samdroid-apps Great suggestions! I try to implement some of these and update you with a new patch ;)

@quozl
Copy link
Contributor

quozl commented Mar 14, 2016

Tested on OLPC XO-1 with Fedora 18.

It is very slow to respond to up and down arrow key presses. Initial delay is about one second. Subsequent delay about a fifth of a second. Perhaps the tree view is being redrawn between movements. Holding the up or down arrow keys, to force autorepeat, consumes 100% CPU, even though the selected item is already at the top or bottom of the list.

Agreed, rename first letter is lost. Space bar does not work as first letter. After renaming, two up keys are needed to get to the entry above; should only be one up key needed.

Agreed, left arrow in addition to escape for backing out of details. While in details, tab did not work for moving focus between description, tags, and comments.

Please add a key for showing the journal entry palette, and add up down key navigation of the list of actions; start, copy to, duplicate, send to, view details, erase.

Please extend this to confirmation prompts; at the prompt for whether to erase, pressing enter resumed the activity.

Please extend this to the home view activity list; key sequence f3 ctrl+2.

@samdroid-apps
Copy link
Contributor

@quozl said:

Please extend this to the home view activity list; key sequence f3 ctrl+2.

Could we make this code into something in the toolkit? I have a list view in Bibliography activity, and I would like as well to add something that lets me keyboard functions to it. I haven't looked at the code, but could it be an api that is simple, eg.

listview = ...

thing = ListViewKeyboardControl(listview)
thing.connect_key('e', __edit_row_cb)
def __edit_row_cb(listview, row):
   # app specific logic

@quozl
Copy link
Contributor

quozl commented Mar 14, 2016

Worth a try.

Once upon a time Gtk+ applications were expected to have keyboard accessibility features built in, and Gtk+ provides an implementation, with the Gtk.Widget focus properties and functions. But in Sugar some of what was coded conflicted with keyboard focus control.

@iamutkarshtiwari
Copy link
Contributor Author

@samdroid-apps I successfully implemented some of the enlisted features->

When I type the letters to rename, it seems to loose the 1st charecter. It seems to go like
I press 1st letter
Enters textbox, averything selected
I press second letter
I have to go back and enter the 1st letter again :(

Maybe the left arrow key would be good to use for going back to the list view from the details (in addition to the escape key)? It is very intuitive (forward to go in, backward to go out)

For the focus indicator I am thinking to change the entry-search.svg color form black to -> yellow or some other bright color. Or is there anything you would suggest?

@quozl
Copy link
Contributor

quozl commented Mar 14, 2016

If you must indicate focus, use a black dotted or dashed rectangle around the text are of the entry widget.

You may have noticed a very restricted set of colours used by Sugar.

This design decision originated in 2006 with the OLPC XO laptop with sunlight readable reflective display, which is colour when transmissive and monochrome when reflective. An additional justification was accessibility for children with colour vision deficiency.

The frequency of colour vision deficiency means it would be best to use black and white rather than yellow.

(Most displays and supporting software remain trichromatic. Some human populations show females with up to 50% tetrachromacy, yet our displays aren't handling them well.)

@iamutkarshtiwari
Copy link
Contributor Author

@quozl Any hints on how to get those black dotted or dashed rectangle around text area?

@quozl
Copy link
Contributor

quozl commented Mar 15, 2016

Perhaps something to do with the CSS in sugar-artwork repository?

@samdroid-apps
Copy link
Contributor

I ran with this patch for a while to test it more. What I found it was that it conflicted with touch-scrolling. When I could touch scroll, it would actually drag entry instead. Please try testing on a touchscreen, or turning on the "Simulate Touchscreen" option under the visual tab of the Gtk+ inspector.

@quozl
Copy link
Contributor

quozl commented Mar 16, 2016

Well spotted. I tested with XO-1, no touch.

@davelab6
Copy link
Contributor

davelab6 commented Apr 4, 2016

@iamutkarshtiwari this is really awesome!! :D Any news? :)

@iamutkarshtiwari
Copy link
Contributor Author

@davelab6 Thanks! :) I fixed some of the issues of this feature as suggested by the community members. As Mr. Sam Parkinson says.. it needs more work so I planning on writing an API to extend keyboard accessibility to the Sugar. I have been busy with the GSoC project so couldn't devote much time to it but will definitely bring this to existence ;)

@icarito
Copy link
Contributor

icarito commented May 23, 2016

This is a cool feature it would be great to make it happen.

@quozl
Copy link
Contributor

quozl commented Nov 9, 2016

Ping?

@i5o
Copy link
Contributor

i5o commented Nov 15, 2016

@iamutkarshtiwari can you update the code to work with the current one? If you need help let me know. (conflicts with src/jarabe/journal/journalactivity.py)

@iamutkarshtiwari
Copy link
Contributor Author

@i5o I am having hard time resolving this conflict :/ Could you please help?

@quozl
Copy link
Contributor

quozl commented Nov 16, 2016

@iamutkarshtiwari the conflict looks quite straightforward to an expert. 😀 Do you mean help with wrangling git, or help understanding how to apply your patch again to the current master branch?

Because your changes are small, it may be easier to start a new branch, based on master, and edit in your changes;

git fetch upstream
git checkout -b feature-journalAccessiblityKeys2 upstream/master
emacs src/jarabe/journal/journalactivity.py

Then to get that branch into this pull request;

git push --force origin feature-journalAccessiblityKeys2:feature-journalAccessiblityKeys

There are many other ways of solving this; it is hard to be definitive.

@iamutkarshtiwari
Copy link
Contributor Author

@quozl Conflict resolved

@quozl
Copy link
Contributor

quozl commented Apr 25, 2017

Thanks, I'll do another test of the changes and get back to you.

GitHub still says "branch cannot be rebased due to conflicts" when "Rebase and merge" is selected, so if you don't rebase I'll be doing it by hand. Really don't want unnecessary merge commits in history, because they make git bisect so much harder.

gitk shows the first commit is parented all the way back to 2016-03-10. 😁

@iamutkarshtiwari
Copy link
Contributor Author

I resolved the merge conflicts from github directly. If there is still such issue, I'll rebase it to the latest branch.

@quozl
Copy link
Contributor

quozl commented Apr 25, 2017

GitHub still says "branch cannot be rebased due to conflicts" when "Rebase and merge" is selected, and further it says "Rebasing the commits of this branch on top of the base branch cannot be performed automatically due to conflicts encountered while reapplying the individual commits from the head branch."

If you're still not convinced, use gitk or another visualisation to see the ancestry of your commits, and you'll agree that the commits cannot be individually applied.

Entries can be scrolled with 'Up'/'Down' arrow keys.
Entry(activity) can be started/resumed by pressing 'Enter' key.
Entry can be renamed by typing anything.
Detail View of an entry can be opened via 'Right' arrow key.
Switching b/w search entry and tree view can done via 'Tab' key.
Escape key can be use to switch back to list from detail view.
@iamutkarshtiwari
Copy link
Contributor Author

@quozl Rebased

@quozl
Copy link
Contributor

quozl commented May 2, 2017

@iamutkarshtiwari, thanks. Have you tested this?

  • I get an error in logs;
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/jarabe/main.py", line 126, in setup_journal_cb
    journalactivity.start()
  File "/usr/lib/python2.7/dist-packages/jarabe/journal/journalactivity.py", line 654, in start
    get_journal()
  File "/usr/lib/python2.7/dist-packages/jarabe/journal/journalactivity.py", line 611, in get_journal
    _journal = JournalActivity()
  File "/usr/lib/python2.7/dist-packages/jarabe/journal/journalactivity.py", line 219, in __init__
    self.connect('key-press-event', self.__key_press_event_cb)
AttributeError: 'JournalActivity' object has no attribute '_JournalActivity__key_press_event_cb'

Just a typo.

  • When I fix that typo, and move down the journal with down arrow, and resume an entry, then stop, then press down arrow once, I get this error;
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/jarabe/journal/listview.py", line 763, in __key_detect_cb
    self.tree_view.set_cursor_on_cell(path, column, None ,  True)
TypeError: Argument 1 does not allow None as a value
  • Spacing on that line of code is wrong, also.

While testing, noticed a few things;

  • pressing left arrow in the detail view gives a sound alert and returns to the list view; while returning to the list view is good, the sound alert is not,
  • while on the detail view, the return key is ineffective; should it resume the entry?
  • pressing tab to get to the search view, then typing b works fine, but focus is stolen and subsequent typing is ignored; the search entry remains set to b only,
  • when renaming, the first character typed is lost, as @samdroid-apps wrote in March 2016.

Other ideas;

  • ticket suggestion asked for change volume; that is switching between journal, Documents, and any mounted filesystems. You might do that with ctrl+1, ctrl+2, etc.
  • not sure I agree with the use of tab; perhaps instead it should be ctrl+f to assign focus to the search entry, and arrow keys or escape to move focus back to the list.

@rahul-bothra
Copy link
Contributor

@iamutkarshtiwari, Great feature
Will you be continuing this or may I work on the same, Thanks

@rahul-bothra
Copy link
Contributor

Can be closed in favor of #791

@quozl
Copy link
Contributor

quozl commented Mar 14, 2018

Thanks. Did you use any of this work? If so, please see if you can split your patches and use "git commit --amend --author" to properly credit @iamutkarshtiwari.

@quozl quozl closed this Mar 15, 2018
@quozl
Copy link
Contributor

quozl commented Mar 15, 2018

I see 2670767 has co-authorship.

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

Successfully merging this pull request may close these issues.

None yet

7 participants