Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Added Code to HtmlUtils to return info when in closing tag #3419

Merged
merged 5 commits into from
Apr 20, 2013
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/editor/CSSInlineEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ define(function (require, exports, module) {
var tagInfo = HTMLUtils.getTagInfo(editor, pos),
selectorName = "";

if (tagInfo.position.tokenType === HTMLUtils.TAG_NAME) {
if (tagInfo.position.tokenType === HTMLUtils.TAG_NAME || tagInfo.position.tokenType === HTMLUtils.CLOSING_TAG) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to add a new unit test for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

You still need to add a unit test for Quick Edit in closing tag to test this specific change, probably in InlineEditorProviders-test.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems i've missed that

// Type selector
selectorName = tagInfo.tagName;
} else if (tagInfo.position.tokenType === HTMLUtils.ATTR_NAME ||
Expand Down
22 changes: 14 additions & 8 deletions src/language/HTMLUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ define(function (require, exports, module) {

//constants
var TAG_NAME = "tagName",
CLOSING_TAG = "closingTag",
ATTR_NAME = "attr.name",
ATTR_VALUE = "attr.value";

Expand Down Expand Up @@ -399,12 +400,16 @@ define(function (require, exports, module) {
return createTagInfo();
}

// Check to see if this is the closing of a tag (either the start or end)
if (ctx.token.string === ">" || ctx.token.string === "/>" ||
(ctx.token.string.charAt(0) === "<" && ctx.token.string.charAt(1) === "/")) {
// Check to see if this is the closing of a tag (either the start or end) self closing tag
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also update this comment to " the closing of a start tag or self closing tag."

if (ctx.token.string === ">" || ctx.token.string === "/>") {
return createTagInfo();
}

// Check to see if this is the closing of a tag (either the start or end) closing tag
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I should have caught this in the previous round of review. This comment is no longer correct. We're just handling closing tag here.

if (ctx.token.string.charAt(0) === "<" && ctx.token.string.charAt(1) === "/") {
return createTagInfo(CLOSING_TAG, offset - 2, ctx.token.string.slice(2));
}

// Make sure the cursor is not after an equal sign or a quote before we report the context as a tag.
if (ctx.token.string !== "=" && ctx.token.string.match(/^["']/) === null) {
if (!tokenType) {
Expand Down Expand Up @@ -504,14 +509,15 @@ define(function (require, exports, module) {


// Define public API
exports.TAG_NAME = TAG_NAME;
exports.ATTR_NAME = ATTR_NAME;
exports.ATTR_VALUE = ATTR_VALUE;
exports.TAG_NAME = TAG_NAME;
exports.CLOSING_TAG = CLOSING_TAG;
exports.ATTR_NAME = ATTR_NAME;
exports.ATTR_VALUE = ATTR_VALUE;

exports.getTagInfo = getTagInfo;
exports.getTagInfo = getTagInfo;
exports.getTagAttributes = getTagAttributes;
//The createTagInfo is really only for the unit tests so they can make the same structure to
//compare results with
exports.createTagInfo = createTagInfo;
exports.createTagInfo = createTagInfo;
exports.findStyleBlocks = findStyleBlocks;
});
20 changes: 10 additions & 10 deletions test/spec/CodeHintUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,16 @@ define(function (require, exports, module) {
expect(tag).toEqual(HTMLUtils.createTagInfo(HTMLUtils.TAG_NAME));
});

it("should not hint anything inside a closing tag", function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

You have moved this unit test to a different location in the same file. Can you restore it to the previous location so that it is next to other related unit tests?

var pos = {"ch": 0, "line": 0};
setContentAndUpdatePos(pos,
["<html>", "<body>", "<div id='test' class='foo'></div>"],
"</body></ht", "ml>");

var tag = HTMLUtils.getTagInfo(myEditor, pos);
expect(tag).toEqual(HTMLUtils.createTagInfo(HTMLUtils.CLOSING_TAG, 2, "html"));
});

it("should find the tagname of the current tag if two tags are right next to each other", function () {
var pos = {"ch": 0, "line": 0};
setContentAndUpdatePos(pos,
Expand Down Expand Up @@ -237,16 +247,6 @@ define(function (require, exports, module) {
expect(tag).toEqual(HTMLUtils.createTagInfo());
});

it("should not hint anything inside a closing tag", function () {
var pos = {"ch": 0, "line": 0};
setContentAndUpdatePos(pos,
["<html>", "<body>", "<div id='test' class='foo'></div>"],
"</body></ht", "ml>");

var tag = HTMLUtils.getTagInfo(myEditor, pos);
expect(tag).toEqual(HTMLUtils.createTagInfo());
});

it("should not find attributes in an empty editor", function () {
var pos = {"ch": 0, "line": 0};
var attrs = HTMLUtils.getTagAttributes(myEditor, pos);
Expand Down
2 changes: 1 addition & 1 deletion test/spec/InlineEditorProviders-test-files/test1.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
<div{{0}} class="{{1}}foo" id="myDiv{{2}}">{{3}}</div>
<!--<div{{4}}>-->
<div id="anotherDiv{{6}}"></div>
<div{{8}} {{7}}class="foo"></div>
<div{{8}} {{7}}class="foo"></{{9}}div>
</body>
</html>
14 changes: 13 additions & 1 deletion test/spec/InlineEditorProviders-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ define(function (require, exports, module) {
});


it("should open a type selector", function () {
it("should open a type selector on opening tag", function () {
initInlineTest("test1.html", 0);

runs(function () {
Expand All @@ -263,6 +263,18 @@ define(function (require, exports, module) {
expect(inlinePos).toEqual(this.infos["test1.css"].offsets[0]);
});
});

it("should open a type selector on closing tag", function () {
initInlineTest("test1.html", 9);

runs(function () {
var inlineWidget = EditorManager.getCurrentFullEditor().getInlineWidgets()[0];
var inlinePos = inlineWidget.editors[0].getCursorPos();

// verify cursor position in inline editor
expect(inlinePos).toEqual(this.infos["test1.css"].offsets[0]);
});
});

it("should open a class selector", function () {
initInlineTest("test1.html", 1);
Expand Down