-
Notifications
You must be signed in to change notification settings - Fork 812
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
[MM-20796] Un-reverted the Ctrl+Tab fix and also made sure the tabs go in the right order #1173
Conversation
…o in the right order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @devinbinnie - I'll test this on the next RC
src/browser/components/MainPage.jsx
Outdated
const nextIndex = this.props.teams.map((team, index) => { | ||
return { | ||
index, | ||
order: (team.order - currentOrder) - 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic here can be made more readable if we swap the parens
order: (team.order - currentOrder) - 1, | |
order: team.order - (currentOrder + 1), |
which looks more like "next tab"
src/browser/components/MainPage.jsx
Outdated
const currentOrder = this.props.teams[this.state.key].order; | ||
const nextIndex = this.props.teams.map((team, index) => { | ||
return { | ||
index, | ||
order: (team.order - currentOrder) + 1, | ||
}; | ||
}).find((team) => team.order === 0 || Math.abs(team.order) === this.props.teams.length); | ||
this.handleSelect(nextIndex.index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comments as above, but going back ;)
@@ -335,7 +335,7 @@ export default class MattermostView extends React.Component { | |||
if (this.props.withTab) { | |||
classNames.push('mattermostView-with-tab'); | |||
} | |||
if (!this.props.active || this.state.errorInfo) { | |||
if (!this.props.active) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure of why this change is here :/ seems unrelated to the PR, can you help me understand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this PR: #1127
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
@@ -335,7 +335,7 @@ export default class MattermostView extends React.Component { | |||
if (this.props.withTab) { | |||
classNames.push('mattermostView-with-tab'); | |||
} | |||
if (!this.props.active || this.state.errorInfo) { | |||
if (!this.props.active) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Co-Authored-By: Guillermo Vayá <guivaya@gmail.com>
gah! hit the button too soon, there is an alternate implementation: ipcRenderer.on('select-next-tab', () => {
const currentOrder = this.props.teams[this.state.key].order;
const nextOrder = ((currentOrder + 1) % this.props.teams.length);
const nextIndex = this.props.teams.findIndex((team) => team.order === nextOrder);
this.handleSelect(nextIndex);
});
ipcRenderer.on('select-previous-tab', () => {
const currentOrder = this.props.teams[this.state.key].order;
// js modulo operator returns a negative number if result is negative, so we have to ensure it's positive
const nextOrder = ((this.props.teams.length + (currentOrder - 1)) % this.props.teams.length);
const nextIndex = this.props.teams.findIndex((team) => team.order === nextOrder);
this.handleSelect(nextIndex);
}); |
@devinbinnie
|
…o in the right order (mattermost#1173) * [MM-20796] Un-reverted the Ctrl+Tab fix and also made sure the tabs go in the right order * Style fixes * Update src/browser/components/MainPage.jsx Co-Authored-By: Guillermo Vayá <guivaya@gmail.com> * Different logic Co-authored-by: Guillermo Vayá <guivaya@gmail.com>
…o in the right order (#1173) (#1175) * [MM-20796] Un-reverted the Ctrl+Tab fix and also made sure the tabs go in the right order * Style fixes * Update src/browser/components/MainPage.jsx Co-Authored-By: Guillermo Vayá <guivaya@gmail.com> * Different logic Co-authored-by: Guillermo Vayá <guivaya@gmail.com> Co-authored-by: Guillermo Vayá <guivaya@gmail.com>
Before submitting, please confirm you've
npm run lint:js
for proper code formattingPlease provide the following information:
Summary
The fix in this PR: #1127
Was overwritten by this PR: #1056
This PR puts the fix back.
Also found a bug where tabbing through when you've re-ordered the tabs would tab through in the wrong order, also fixed.
Issue link
https://mattermost.atlassian.net/browse/MM-20796