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.

Sign up for GitHub

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

Jump to bottom

Convert Theme Package to JS #6491

Merged
merged 7 commits into from
Mar 29, 2017
Merged

Convert Theme Package to JS #6491

merged 7 commits into from
Mar 29, 2017

Conversation

MartinSchoeler
Copy link
Contributor

@RocketChat/core

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6491 March 27, 2017 13:08 Inactive
rodrigok
rodrigok requested changes Mar 28, 2017
View reviewed changes
packages/rocketchat-theme/server/server.js Outdated

const less = Npm.require('less');
const Autoprefixer = Npm.require('less-plugin-autoprefix');
const crypto = Npm.require('crypto');
Copy link
Member

Choose a reason for hiding this comment

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

Use import

packages/rocketchat-theme/server/server.js Outdated
const prefix = __meteor_runtime_config__.ROOT_URL_PATH_PREFIX || '';
if (path === (`${ prefix }/__cordova/theme.css`) || path === (`${ prefix }/theme.css`)) {
css = RocketChat.theme.getCss();
hash = crypto.createHash('sha1').update(css).digest('hex');
Copy link
Member

Choose a reason for hiding this comment

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

Declare css and hash here

packages/rocketchat-theme/server/server.js Outdated

const path = req.url.split('?')[0];
const prefix = __meteor_runtime_config__.ROOT_URL_PATH_PREFIX || '';
if (path === (`${ prefix }/__cordova/theme.css`) || path === (`${ prefix }/theme.css`)) {
Copy link
Member

Choose a reason for hiding this comment

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

No need for these parenthesis

packages/rocketchat-theme/server/server.js Outdated
const css = RocketChat.theme.getCss();
if (css.trim() !== '') {
hash = crypto.createHash('sha1').update(css).digest('hex');
themeManifestItem = _.find(manifest, function(item) {
Copy link
Member

Choose a reason for hiding this comment

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

Declare hash and themeManifestItem here

packages/rocketchat-theme/server/server.js Outdated
RocketChat.settings.add('css', '');
RocketChat.settings.addGroup('Layout');
RocketChat.settings.onload('css', Meteor.bindEnvironment(() => {
return function(key, value, initialLoad) {
Copy link
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 for this function, pass the params directly to the above arrow function

packages/rocketchat-theme/server/server.js Outdated
value
};
if (persist === true) {
config = {
Copy link
Member

Choose a reason for hiding this comment

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

Declare config here

packages/rocketchat-theme/server/server.js Outdated
addPublicColor(name, value, section, editor) {
if (editor == null) {
editor = 'color';
}
Copy link
Member

Choose a reason for hiding this comment

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

Use default values

packages/rocketchat-theme/server/server.js Outdated
}

getVariablesAsLess() {
return Object.keys(this.variables).map((obj, name) => {
Copy link
Member

Choose a reason for hiding this comment

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

map((obj, name) => { is wrong, the correct is map((name) => {

packages/rocketchat-theme/server/variables.js Outdated
// Bulk-add settings for color scheme
let value;
Object.keys(majorColors).forEach((key) => {
value = majorColors[key];
Copy link
Member

Choose a reason for hiding this comment

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

Declare value here using const

packages/rocketchat-theme/server/variables.js Outdated
});

Object.keys(minorColors).forEach((key) => {
value = minorColors[key];
Copy link
Member

Choose a reason for hiding this comment

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

Declare value here using const

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6491 March 29, 2017 19:02 Inactive
rodrigok
rodrigok requested changes Mar 29, 2017
View reviewed changes
packages/rocketchat-theme/server/server.js Outdated
this.variables[name] = {
type,
value
};
if (persist === true) {
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this if?

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6491 March 29, 2017 19:43 Inactive
@rodrigok rodrigok modified the milestone: 0.55.0 Mar 29, 2017
@rodrigok rodrigok merged commit 25ff24a into develop Mar 29, 2017
@rodrigok rodrigok deleted the theme-to-js branch March 29, 2017 21:04
Syirrus pushed a commit to Syirrus/Rocket.Chat that referenced this pull request Mar 31, 2017
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
47157e9 
…nto clarapy-v4

Changed over home.coffee & help.coffee to .js

* 'develop' of https://github.com/RocketChat/Rocket.Chat: (162 commits)
  Remove coffeescript package from ui-flextab
  Remove coffeescript package from ui-sidenav
  Update PULL_REQUEST_TEMPLATE.md
  Fix Outgoing Webhooks Retrying Not Respecting Enabled Status (RocketChat#6478)
  Convert Theme Package to JS (RocketChat#6491)
  Fix typo of the safari pinned tab label (RocketChat#6487)
  fix channel merge option of user preferences (RocketChat#6493)
  converted Rocketchat logger coffee to js (RocketChat#6495)
  converted rocketchat-integrations coffee to js (RocketChat#6502)
  'allow reacting' should be a toggle option (RocketChat#6522)
  Add to HISTORY.md
  Fix backup codes remaining not showing
  Fix ESLint
  Fix ESLint
  Fix review
  Fix review
  Fix review
  fix import issue
  fix
  fix
  ...

# Conflicts:
#	packages/rocketchat-lib/server/lib/PushNotification.js
#	packages/rocketchat-ui/client/views/app/home.coffee
#	packages/rocketchat-ui/package.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rodrigok rodrigok rodrigok approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.55.0
Development

Successfully merging this pull request may close these issues.
4 participants
@MartinSchoeler @rodrigok @engelgabriel @marceloschmidt