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

Convert Ui Account Package to Js #6795

Merged
merged 8 commits into from May 16, 2017
Merged

Convert Ui Account Package to Js #6795

merged 8 commits into from May 16, 2017

Conversation

MartinSchoeler
Copy link
Contributor

@RocketChat/core

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6795 April 25, 2017 17:48 Inactive
const languages = TAPi18n.getLanguages();
const result = [];

Object.keys(languages).forEach((key) => {
Copy link
Member

Choose a reason for hiding this comment

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

use .map

checked(property, value, defaultValue) {
const user = Meteor.user();
let currentValue;
if (user && user.settings && user.settings.preferences && user.settings.preferences[property] && defaultValue === true) {
Copy link
Member

Choose a reason for hiding this comment

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

something like that is better:

checked(property, value, defaultValue) {
    const user = Meteor.user();
    if (user && user.settings && user.settings.preferences && user.settings.preferences[property]) {
        if(defaultValue === true) {
            return currentValue === value;
        }
        return !!user.settings.preferences[property] === value
    }
},

},
selected(property, value, defaultValue) {
const user = Meteor.user();
if (!(user && user.settings && user.settings.preferences && user.settings.preferences[property])) {
Copy link
Member

Choose a reason for hiding this comment

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

You can test this better if you invert the logic and test just one time user && user.settings && user.settings .....

return user && user.settings && user.settings.preferences && user.settings.preferences['highlights'] && user.settings.preferences['highlights'].join(', ');
},
desktopNotificationEnabled() {
return (KonchatNotification.notificationStatus.get() === 'granted') || (window.Notification && Notification.permission === 'granted');
Copy link
Member

Choose a reason for hiding this comment

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

you can remove (...)

return (KonchatNotification.notificationStatus.get() === 'granted') || (window.Notification && Notification.permission === 'granted');
},
desktopNotificationDisabled() {
return (KonchatNotification.notificationStatus.get() === 'denied') || (window.Notification && Notification.permission === 'denied');
Copy link
Member

Choose a reason for hiding this comment

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

you can remove (...)

confirmButtonText: t('Delete'),
cancelButtonText: t('Cancel')
}, () => {
return function(typedPassword) {
Copy link
Member

Choose a reason for hiding this comment

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

you are returning a function that never will be executed.

confirmButtonText: t('Delete'),
cancelButtonText: t('Cancel')
}, () => {
return function(deleteConfirmation) {
Copy link
Member

Choose a reason for hiding this comment

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

you are returning a function that never will be executed.

e.currentTarget.innerHTML = `${ e.currentTarget.innerHTML } ...`;
e.currentTarget.disabled = true;
return Meteor.call('sendConfirmationEmail', user.emails && user.emails[0] && user.emails[0].address(() => {
return function(error, results) {
Copy link
Member

Choose a reason for hiding this comment

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

you are returning a function that never will be executed.

Template.avatar.helpers({
imageUrl() {
let username = this.username;
if ((username == null) && (this.userId != null)) {
Copy link
Member

Choose a reason for hiding this comment

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

remove ( ... )

const loginWithService = `loginWith${ _.capitalize(this) }`;
const serviceConfig = {};
return Meteor[loginWithService](serviceConfig, function(error) {
if ((error && error.error) === 'github-no-public-email') {
Copy link
Member

Choose a reason for hiding this comment

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

if (error) {

  if(error.error === 'github-no-public-email') {
    return alert(t('github_no_public_email'));
  
  }
  console.log(error)
  return toastr.error(error.message);
}

@MartinSchoeler
Copy link
Contributor Author

I will Improve some things pointed by @ggazzo

@MartinSchoeler
Copy link
Contributor Author

@ggazzo or @rodrigok could you do a review?

@rodrigok rodrigok added this to the 0.57.0 milestone May 16, 2017
@rodrigok rodrigok merged commit d181525 into develop May 16, 2017
@rodrigok rodrigok deleted the ui-account-to-js branch May 16, 2017 14:50
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.

None yet

4 participants