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.

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

[Editor] Add the possibility to create a new editor in using the keyboard (bug 1853424) #16761

Merged
merged 1 commit into from
Oct 6, 2023
Merged

[Editor] Add the possibility to create a new editor in using the keyboard (bug 1853424) #16761

merged 1 commit into from
Oct 6, 2023

Conversation

calixteman
Copy link
Contributor

@calixteman calixteman commented Jul 27, 2023 •
edited
Loading

When an editing button is disabled, focused and the user press Enter (or space), an editor is automatically added at the center of the current page. Next creations can be done in using the same keys within the focused page.

@calixteman calixteman changed the title [Editor] Add the possibility to create a new editor in using the keyb… [Editor] Add the possibility to create a new editor in using the keyboard (bug 1845088) Jul 27, 2023
@calixteman calixteman force-pushed the editor_add_new_with_keyboard branch 3 times, most recently from 8e9883b to db41c69 Compare August 1, 2023 15:20
@marco-c marco-c changed the title [Editor] Add the possibility to create a new editor in using the keyboard (bug 1845088) [Editor] Add the possibility to create a new editor in using the keyboard (bug 1853424) Sep 15, 2023
Snuffleupagus
Snuffleupagus requested changes Oct 4, 2023
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delay, this PR slipped my mind.

src/display/editor/annotation_editor_layer.js Outdated Show resolved Hide resolved
src/display/editor/tools.js Outdated Show resolved Hide resolved
src/display/editor/tools.js Outdated Show resolved Hide resolved
test/integration/freetext_editor_spec.js Outdated Show resolved Hide resolved
@calixteman calixteman force-pushed the editor_add_new_with_keyboard branch 2 times, most recently from 40a0818 to 552cc05 Compare October 5, 2023 17:28
@Snuffleupagus

This comment was marked as outdated.

Sign in to view
@calixteman
Copy link
Contributor Author

It seems that this will make it more difficult for the user to disable editing with the keyboard:

* Tab to the FreeText-button.

* Press `Enter` to enable FreeText-editing.

* Decide that you don't want to add any text and press `Escape`.

* Try to _disable_ FreeText-editing by pressing `Enter` again.

* You're again prompted to add a new FreeText-annotation, with no good way to actually disable the feature.

In this case you need to move the focus back to the freetext button and hit enter.
What do you mean by more difficult ? right now, the only way to disable the feature with the keyboard is move the focus on the button and then press enter. Am I missing something ?

@calixteman
[Editor] Add the possibility to create a new editor in using the keyb…
ea5eafa 
…oard (bug 1853424)

When an editing button is disabled, focused and the user press Enter (or space), an
editor is automatically added at the center of the current page.
Next creations can be done in using the same keys within the focused page.
@Snuffleupagus
Copy link
Collaborator

In this case you need to move the focus back to the freetext button and hit enter.

Sorry, then I apparently misunderstood how all this was supposed to work :-)

@Snuffleupagus
Copy link
Collaborator

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/139f209dbd7e7ec/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/139f209dbd7e7ec/output.txt

Total script time: 1.40 mins

Published

Snuffleupagus
Snuffleupagus approved these changes Oct 6, 2023
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me, thanks!

@calixteman
Copy link
Contributor Author

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 1

Live output at: http://54.193.163.58:8877/77cebfc4605d44e/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/c75102c28ff8a53/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/c75102c28ff8a53/output.txt

Total script time: 5.53 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/77cebfc4605d44e/output.txt

Total script time: 26.08 mins

  • Integration Tests: Passed

@calixteman calixteman merged commit 905ad1f into mozilla:master Oct 6, 2023
3 checks passed
@calixteman calixteman deleted the editor_add_new_with_keyboard branch October 6, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Snuffleupagus Snuffleupagus Snuffleupagus approved these changes

Assignees
No one assigned
Labels
editor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@calixteman @Snuffleupagus @moz-tools-bot