Project

General

Profile

Actions

Feature #34549

closed

Add keyboard shortcuts for wiki toolbar buttons

Added by Marius BĂLTEANU almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
UI
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed

Description

Add keyboard shortcuts for the following buttons from wiki toolbar:

Action Keyboard shortcut
Add bold Ctrl/⌘ + b
Add italic Ctrl/⌘ + i
Add underline Ctrl/⌘ + u

⌘: should work only on osx
Ctrl: should work only on windows/linux.

Any other proposals are welcome!


Files


Related issues

Blocked by Redmine - Feature #30459: Switch edit/preview tabs with keyboard shortcutsClosedGo MAEDA

Actions
Actions #1

Updated by Marius BĂLTEANU almost 4 years ago

  • Subject changed from Add keyboard shortcuts for wiki toolbar to Add keyboard shortcuts for wiki toolbar buttons
Actions #2

Updated by Marius BĂLTEANU almost 4 years ago

  • Description updated (diff)
Actions #3

Updated by Go MAEDA almost 4 years ago

I like the feature. Since the key combination ⌘ + b is available in so many apps, I sometimes mistakenly use the key combination in Redmine.

Ctrl: should work only on windows/linux.

It is really important. Ctrl + B is already used as a shortcut key of the left arrow key in macOS.

Actions #4

Updated by Marius BĂLTEANU almost 4 years ago

  • File 0001-Add-keyboard-shortcuts-for-bold-italic-and-underline.patch added

Here is an almost ready version of the patch for testing purposes. Must be applied on top of #30459.

I need to test the feature on multiple browsers/devices, add system tests and improve the code if possible.

Actions #5

Updated by Marius BĂLTEANU almost 4 years ago

  • Blocked by Feature #30459: Switch edit/preview tabs with keyboard shortcuts added
Actions #6

Updated by Marius BĂLTEANU almost 4 years ago

  • File deleted (0001-Add-keyboard-shortcuts-for-bold-italic-and-underline.patch)
Actions #7

Updated by Marius BĂLTEANU almost 4 years ago

  • File 0001-Add-keyboard-shortcuts-for-bold-italic-and-underline.patch added
Actions #8

Updated by Marius BĂLTEANU almost 4 years ago

  • File deleted (0001-Add-keyboard-shortcuts-for-bold-italic-and-underline.patch)
Actions #10

Updated by Marius BĂLTEANU almost 4 years ago

  • Assignee deleted (Marius BĂLTEANU)
Actions #11

Updated by Go MAEDA almost 4 years ago

  • Category set to UI
  • Target version changed from Candidate for next major release to 4.2.0

I tried out the patch and found it works fine and is quite useful.

Setting the target version to 4.2.0.

Actions #12

Updated by Go MAEDA almost 4 years ago

Thank you for writing the patch but I found some tests fail on macOS. This is because the tests use a control key. As written in the description, control key does not work on macOS.

laphroaig:redmine-trunk maeda$ ruby test/system/keyboard_shortcuts_test.rb
Run options: --seed 58685

# Running:

Capybara starting Puma...
* Version 5.1.1 , codename: At Your Service
* Min threads: 0, max threads: 4
* Listening on http://127.0.0.1:64266
[Screenshot]: tmp/screenshots/failures_test_keyboard_shortcuts_for_wiki_toolbar_buttons_using_textile.png
F

Failure:
InlineAutocompleteSystemTest#test_keyboard_shortcuts_for_wiki_toolbar_buttons_using_textile [test/system/keyboard_shortcuts_test.rb:78]:
Expected: "**" 
  Actual: "" 

bin/rails test test/system/keyboard_shortcuts_test.rb:72

[Screenshot]: tmp/screenshots/failures_test_keyboard_shortcuts_keys_should_be_shown_in_button_title.png
F

Failure:
InlineAutocompleteSystemTest#test_keyboard_shortcuts_keys_should_be_shown_in_button_title [test/system/keyboard_shortcuts_test.rb:117]:
Expected: "Strong (Ctrl+B)" 
  Actual: "Strong (⌘B)" 

bin/rails test test/system/keyboard_shortcuts_test.rb:112

.[Screenshot]: tmp/screenshots/failures_test_keyboard_shortcuts_for_wiki_toolbar_buttons_using_markdown.png
F

Failure:
InlineAutocompleteSystemTest#test_keyboard_shortcuts_for_wiki_toolbar_buttons_using_markdown [test/system/keyboard_shortcuts_test.rb:98]:
Expected: "****" 
  Actual: "" 

bin/rails test test/system/keyboard_shortcuts_test.rb:92

.

Finished in 14.276589s, 0.3502 runs/s, 0.9106 assertions/s.
5 runs, 13 assertions, 3 failures, 0 errors, 0 skips
Actions #13

Updated by Marius BĂLTEANU almost 4 years ago

Thanks Go Maeda for catching this issue, I had in mind only to pass on Linux instances (I've docker on my local env). Updated a new version of the patch that should pass on OSX, but I didn't test it, please let me know how it goes.

The new version adds the following changes:
1. Adds a new method osx? on lib/platform that returns true if RUBY_PLATFORM is darwin.
2. In the tests, the modifier key is returned based on osx?.

Actions #14

Updated by Go MAEDA almost 4 years ago

Marius BALTEANU wrote:

The new version adds the following changes:
1. Adds a new method osx? on lib/platform that returns true if RUBY_PLATFORM is darwin.
2. In the tests, the modifier key is returned based on osx?.

Thank you, it now passes all tests also on macOS. I will commit the patch soon.

Actions #15

Updated by Go MAEDA almost 4 years ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch. Thank you.

Actions #16

Updated by Go MAEDA almost 4 years ago

After r20729, tooltips for the buttons always displaying in English regardless of the current user's Language.

The following code fixes the issue.

diff --git a/public/javascripts/jstoolbar/jstoolbar.js b/public/javascripts/jstoolbar/jstoolbar.js
index ed6605965..04c35fa95 100644
--- a/public/javascripts/jstoolbar/jstoolbar.js
+++ b/public/javascripts/jstoolbar/jstoolbar.js
@@ -250,10 +250,16 @@ jsToolBar.prototype = {
     return b;
   },
   buttonTitleWithShortcut: function(title, shortcutKey) {
+    if(typeof jsToolBar.strings == 'undefined') {
+      var translatedTitle = title || null;
+    } else {
+      var translatedTitle = jsToolBar.strings[title] || title || null;
+    }
+
     if (isMac) {
-      return title + " (⌘" + shortcutKey.toUpperCase() + ")";
+      return translatedTitle + " (⌘" + shortcutKey.toUpperCase() + ")";
     } else {
-      return title + " (Ctrl+" + shortcutKey.toUpperCase() + ")";
+      return translatedTitle + " (Ctrl+" + shortcutKey.toUpperCase() + ")";
     }
   },
   space: function(toolName) {
Actions #17

Updated by Go MAEDA almost 4 years ago

  • Status changed from Reopened to Closed

Go MAEDA wrote:

After r20729, tooltips for the buttons always displaying in English regardless of the current user's Language.

Committed the fix in r20743.

Actions

Also available in: Atom PDF