Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29723623: Issue 6487 - IOElement as base component for ABP UI (Closed)

Created:
March 15, 2018, 2:42 p.m. by a.giammarchi
Modified:
April 3, 2018, 5:58 p.m.
Reviewers:
saroyanm
CC:
Thomas Greiner
Visibility:
Public.

Description

Issue 6487 - IOElement as base component for ABP UI

Patch Set 1 #

Total comments: 9

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+230 lines, -2 lines) Patch
M .gitignore View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M .hgignore View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M README.md View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
A js/io-element.js View 1 chunk +60 lines, -0 lines 0 comments Download
M package.json View 1 2 1 chunk +7 lines, -1 line 0 comments Download
A tests/README.md View 1 chunk +48 lines, -0 lines 0 comments Download
A tests/background.html View 1 chunk +30 lines, -0 lines 0 comments Download
A tests/io-element.html View 1 chunk +35 lines, -0 lines 0 comments Download
A tests/io-element.js View 1 2 1 chunk +28 lines, -0 lines 0 comments Download
A tests/locale/en_US/common.json View 1 chunk +1 line, -0 lines 0 comments Download
A tests/locale/en_US/io-element.json View 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 8
a.giammarchi
Ready for review. My only doubt is if we want to include the generated test ...
March 19, 2018, 11:43 a.m. (2018-03-19 11:43:59 UTC) #1
saroyanm
Looks to be a great start, I only have couple of question, but in overal ...
March 20, 2018, 8:08 p.m. (2018-03-20 20:08:19 UTC) #2
a.giammarchi
I've answered Manvel, going to update the diff removing the test file. https://codereview.adblockplus.org/29723623/diff/29723624/js/io-element.js File js/io-element.js ...
March 21, 2018, 7:27 a.m. (2018-03-21 07:27:19 UTC) #3
a.giammarchi
Removed all test files (.gitignore) and manually removed tests/io-element.js in mercurial due its inability to ...
March 21, 2018, 8:11 a.m. (2018-03-21 08:11:32 UTC) #4
a.giammarchi
Explained some extra detail about the unique ID choice. https://codereview.adblockplus.org/29723623/diff/29723624/js/io-element.js File js/io-element.js (right): https://codereview.adblockplus.org/29723623/diff/29723624/js/io-element.js#newcode20 js/io-element.js:20: ...
March 21, 2018, 8:15 a.m. (2018-03-21 08:15:57 UTC) #5
saroyanm
This is LGTM to start using hyperhtml, I still need to go through whole documentation ...
March 21, 2018, 11:22 a.m. (2018-03-21 11:22:22 UTC) #6
a.giammarchi
Updated HyperHTMLElement documentation + code (to point at such documentation). Using a smoke folder for ...
March 21, 2018, 2:21 p.m. (2018-03-21 14:21:32 UTC) #7
saroyanm
March 21, 2018, 4:11 p.m. (2018-03-21 16:11:35 UTC) #8
LGTM

Powered by Google App Engine
This is Rietveld