Skip to content

Convert Ui-Master Package to Js#6498

Merged
engelgabriel merged 3 commits into
developfrom
master-to-js
Mar 29, 2017
Merged

Convert Ui-Master Package to Js#6498
engelgabriel merged 3 commits into
developfrom
master-to-js

Conversation

@MartinSchoeler

Copy link
Copy Markdown
Member

@RocketChat/core

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6498 March 27, 2017 17:25 Inactive
@@ -0,0 +1,303 @@
/* globals toolbarSearch, menu, isRtl, fireGlobalEvent, CachedChatSubscription */
Template.body.onRendered(function() {
$(document.body).on('keydown', function(e) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happened with clipboard?

@MartinSchoeler MartinSchoeler Mar 28, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@rodrigok It was never used again, breaking a eslint rule

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, so remove the variable assign, but keep the Clipboard instantiation

ls: 1
}
});
fetch = subscriptions.fetch();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where did you declare this var?

}
});
fetch = subscriptions.fetch();
const results = [];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is no need to get and return results here

fetch = subscriptions.fetch();
const results = [];

Object.keys(fetch).forEach((key) =>{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The fetch's result is not an object, it's an array

},
requirePasswordChange() {
const user = Meteor.user();
return (user ? user.requirePasswordChange : null) === true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can simplify:

return user && user.requirePasswordChange === true;

return;
}
t.touchstartX = null;
t.touchstartY = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you change from undefined to null?

},
'touchmove'(e, t) {
if (t.touchstartX != null) {
const touch = e.originalEvent.touches[0];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can use const [touch] = e.originalEvent.touches; here

cancelButtonText: t('Cancel')
});
const user = Meteor.user();
const settings = user.settings;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

user can be undefined

});
const user = Meteor.user();
const settings = user.settings;
const prefs = user ? settings ? settings.preferences : null : null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplify with const prefs = settings && settings.preferences

const user = Meteor.user();
const settings = user.settings;
const prefs = user ? settings ? settings.preferences : null : null;
if (prefs != null ? prefs.hideUsernames : null) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplify with if (prefs && prefs.hideUsernames) {

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6498 March 29, 2017 16:37 Inactive
@engelgabriel engelgabriel added this to the 0.55.0 milestone Mar 29, 2017
@engelgabriel engelgabriel merged commit 55b14e3 into develop Mar 29, 2017
@engelgabriel engelgabriel deleted the master-to-js branch March 29, 2017 18:26
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.

4 participants