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

html-reporter: Made background switchable #143

Merged
merged 1 commit into from
Apr 15, 2015
Merged

html-reporter: Made background switchable #143

merged 1 commit into from
Apr 15, 2015

Conversation

unlok
Copy link

@unlok unlok commented Apr 10, 2015

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.5% when pulling 82aaac6 on background into bb587e3 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.5% when pulling 82aaac6 on background into bb587e3 on master.

@unlok unlok changed the title html-reporter: Fix inlined SVG html-reporter: Made background switchable Apr 14, 2015
@unlok
Copy link
Author

unlok commented Apr 14, 2015

@SevInf, проверь, пожалуйста. Я так и не могу получить отчет из-за:

JSONWire Error: Unterminated string at character 8

@SevInf
Copy link
Contributor

SevInf commented Apr 14, 2015

@unlok нужен ребейз. И еще jscss ругается на кодстайл.

@@ -19,6 +19,11 @@

{{#if success}}
<div class="image-box">
<div class="cswitcher">
Copy link
Contributor

Choose a reason for hiding this comment

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

data-id же не используется в коде?

@SevInf
Copy link
Contributor

SevInf commented Apr 14, 2015

А можешь скриншотов для истории приложить?

handleColorSwitch(
target,
filter.call(target.parentNode.childNodes, function(node) {
return node.nodeType === 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

вместо magic numbers лучше использовать именованные константы. Здесь Node.ELEMENT_NODE

@unlok
Copy link
Author

unlok commented Apr 14, 2015

Выглядит так:
screen shot 2015-04-14 at 18 20 44

@@ -26,10 +27,39 @@
});
}

function handleColorSwitch(target, sources) {
var imageBox = target.parentNode.parentNode;
Copy link
Contributor

Choose a reason for hiding this comment

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

Как-то хрупко выглядит. Может лучше сразу искать ноду с классом .image-box, чтоб не развалилось при смене верстки

@j0tunn
Copy link
Contributor

j0tunn commented Apr 14, 2015

В остальном 🆗

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.5% when pulling 60fe50d on background into 3f13891 on master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.5% when pulling 60fe50d on background into 3f13891 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.5% when pulling 60fe50d on background into 3f13891 on master.

@unlok
Copy link
Author

unlok commented Apr 15, 2015

@j0tunn @SevInf 🆙

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.5% when pulling 491aa58 on background into 3f13891 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.5% when pulling 491aa58 on background into 3f13891 on master.

SevInf pushed a commit that referenced this pull request Apr 15, 2015
html-reporter: Made background switchable
@SevInf SevInf merged commit 8a89a6a into master Apr 15, 2015
@SevInf SevInf deleted the background branch April 15, 2015 10:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants