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

combine all editors to a single mega-editor #134

Merged
merged 12 commits into from
May 25, 2016
Merged

Conversation

thatkookooguy
Copy link
Member

@thatkookooguy thatkookooguy commented May 22, 2016

Change Summary

es6 isn't supported by ace.js yet. Basically, it's almost identical to javascript with just a few extra things (based on this)

More info

BASE: https://kibibit-demo.herokuapp.com/
MODIFIED: https://kibibit-demo-pr-134.herokuapp.com/

Scenario to check:
try and change existing file types to json or markdown, try opening a file in one of those formats, and try changing the style of the default (no file) document

Before you submit a PR, make sure you did the following things:

  • did you link this PR to an issue?
  • did you lint your changes to both javascript and scss?
  • "I'm pretty sure I'll be able to read and understand this PR, even if I wasn't the author." - _said the PR author_

@@ -9,6 +9,22 @@ kb-code-editor {
.editor {
height: 100%;
width: 100%;
&.half-editor {

Choose a reason for hiding this comment

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

Rule declaration should be preceded by an empty line

@thatkookooguy thatkookooguy requested a deployment to kibibit-demo-pr-134 May 22, 2016 21:16 Pending
@thatkookooguy thatkookooguy requested a deployment to kibibit-demo-pr-134 May 22, 2016 21:56 Pending
});
}
};

vm.shouldShowCompiledView = function() {
vm.showCompiledView = editorSettings.syntaxMode === 'json' || editorSettings.syntaxMode === 'markdown';

Choose a reason for hiding this comment

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

Line must be at most 80 characters

@thatkookooguy thatkookooguy requested a deployment to kibibit-demo-pr-134 May 22, 2016 21:56 Pending
@thatkookooguy thatkookooguy requested a deployment to kibibit-demo-pr-134 May 22, 2016 21:56 Pending
@thatkookooguy thatkookooguy requested a deployment to kibibit-demo-pr-134 May 22, 2016 21:57 Pending
@thatkookooguy thatkookooguy requested a deployment to kibibit-demo-pr-134 May 22, 2016 21:57 Pending
@thatkookooguy thatkookooguy requested a deployment to kibibit-demo-pr-134 May 22, 2016 21:58 Pending
@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-134 May 22, 2016 22:00 Inactive
@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-134 May 22, 2016 22:35 Inactive
});
}
};
});

Choose a reason for hiding this comment

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

Missing line feed at file end

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-134 May 22, 2016 22:37 Inactive
@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-134 May 22, 2016 22:42 Inactive
@thatkookooguy thatkookooguy self-assigned this May 22, 2016
@ortichon
Copy link
Member

When we have empty object viewed in JSON editor, the text is dark on dark, which makes it hard to read.

@ortichon
Copy link
Member

ortichon commented May 25, 2016

LGTM 💯
👯

Approved with PullApprove

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-134 May 25, 2016 18:28 Inactive
top: 0;

.json-formatter-row {
color: white;

Choose a reason for hiding this comment

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

Color white should be written in hexadecimal form as #ffffff
Color literals like white should only be used in variable declarations; they should be referred to via variable everywhere else.

@ortichon
Copy link
Member

ortichon commented May 25, 2016

LGTM
(again)

Approved with PullApprove

@ortichon ortichon merged commit e1143c4 into master May 25, 2016
@ortichon ortichon deleted the combine-all-editors branch May 25, 2016 18:43
neilkalman-redkix pushed a commit that referenced this pull request Jul 4, 2016
combine all editors to a single mega-editor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Syntax highlight returns to 'Text' mode Add temporary es6 support combine editors
4 participants