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

[NEW] Adds a Keyboard Shortcut option to the flextab #5902

Merged
merged 11 commits into from
Aug 22, 2017

Conversation

cnash
Copy link
Contributor

@cnash cnash commented Feb 3, 2017

@RocketChat/core

Closes #46

I added a button to the "FlexTabBar" on the side that opens a tab for displaying the Keyboard Shortcuts. The shortcuts are defined in en.i18n.json (sorry, I only know english). They are in a list, so you can add more in the future without modifying the template.

@CLAassistant
Copy link

CLAassistant commented Feb 3, 2017

CLA assistant check
All committers have signed the CLA.

@rodrigok
Copy link
Member

rodrigok commented Feb 3, 2017

Hi @cnash, can you paste screen shots of your implementation here?

@cnash
Copy link
Contributor Author

cnash commented Feb 3, 2017

screen shot 2017-02-03 at 4 55 12 pm

@engelgabriel engelgabriel added this to the 0.52.0 milestone Feb 6, 2017
{ "Description": "Move to the end of the message", "Keys": "<kbd>Command</kbd> (or <kbd>Alt</kbd>) + <kbd>Down Arrow</kbd>"},
{ "Description": "New line in message compose input", "Keys": "<kbd>Shift</kbd> + <kbd>Enter</kbd>"}
]
},
Copy link
Member

Choose a reason for hiding this comment

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

Heads up! We never used structured json for translations. Not sure how Lingohub will behave.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do I need to change this then? I was hoping to use the list structure to make it easier to add more shortcuts in the future.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure :( let's see if someone has more info on this one regarding lingohub.

Copy link
Member

Choose a reason for hiding this comment

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

I think we will need to change this, LingoHub won't parse this.

Copy link
Member

Choose a reason for hiding this comment

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

Let's make it a fixed list, with unique keys, rather than a loop to iterate over an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay

@engelgabriel engelgabriel modified the milestones: 0.52.0, 0.53.0 Feb 14, 2017
@engelgabriel engelgabriel modified the milestones: 0.54.0, 0.53.0 Mar 2, 2017
@engelgabriel engelgabriel changed the title Fixes #46 - Adds a Keyboard Shortcut option to the flextab Adds a Keyboard Shortcut option to the flextab Mar 2, 2017
Copy link
Member

@marceloschmidt marceloschmidt left a comment

Choose a reason for hiding this comment

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

Change the tree-structured i18n json to fixed list with unique keys.

@engelgabriel engelgabriel modified the milestones: 0.54.0, 0.55.0 Mar 22, 2017
@karlprieb
Copy link
Contributor

@cnash Hey! Do you still have interest in finishing this PR?

@cnash
Copy link
Contributor Author

cnash commented Apr 4, 2017

Yeah. Sorry. Work got crazy, and then I forgot about this. I'll try to fix it up this evening.

@cnash cnash force-pushed the develop branch 2 times, most recently from 35f30ef to 9a83a8a Compare April 5, 2017 13:36
@engelgabriel engelgabriel modified the milestones: 0.55.0, 0.56.0 Apr 13, 2017
@engelgabriel engelgabriel modified the milestones: 0.56.0, 0.55.0 Apr 13, 2017
Modified localized strings to use individual keys rather than structured data.
Moved flex-tab template for keyboard shortcuts because another developer moved the other flex-tab templates.
icon: 'icon-keyboard',
template: 'keyboardShortcuts',
order: 4
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I keep adding newlines to this file, and @codacy-bot keeps complaining.

@engelgabriel engelgabriel modified the milestones: 0.57.0, 0.58.0 Jun 19, 2017
@rodrigok rodrigok modified the milestones: 0.58.0, 0.59.0 Aug 1, 2017
# Conflicts:
#	packages/rocketchat-theme/client/imports/general/base_old.css
groups: ['channel', 'privategroup', 'directmessage'],
id: 'keyboard-shortcut-list',
i18nTitle: 'Keyboard_Shortcuts.Title',
icon: 'icon-keyboard',
Copy link
Member

Choose a reason for hiding this comment

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

@karlprieb Please fix this icon on new design

@rodrigok rodrigok changed the title Adds a Keyboard Shortcut option to the flextab [NEW] Adds a Keyboard Shortcut option to the flextab Aug 22, 2017
@rodrigok rodrigok merged commit 78087a8 into RocketChat:develop Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Help / Shortcuts modal window
6 participants