Add declarative support

[rogozinds]

This change is 

[Artur-]

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions.

---

addon/src/main/java/com/vaadin/board/Board.java, line 86 at r1 (raw file):

        row.setParent(this);
      } else {
        throw new DesignException("Board can have only Row as a child component.");

should Row be <vaadin-board-row> or whatever you have in the declarative file?

---

addon/src/main/java/com/vaadin/board/Row.java, line 180 at r1 (raw file):

            int cols = 1;
            for (Attribute attr : childComponent.attributes()) {
                if (attr.getKey().equals("board-cols")) {

Shouldn't this be :board-cols to be compatible with the spec that says unprefixed attributes belong to the element they are added to and a ":" prefix indicates they belong to the parent. That also makes the custom board- prefix unnecessary so it can be just :cols

---

addon/src/main/java/com/vaadin/board/declarative/BoardComponentMapper.java, line 10 at r1 (raw file):

import sun.reflect.generics.reflectiveObjects.NotImplementedException;

public class BoardComponentMapper extends Design.DefaultComponentMapper {

What is this class for?

---

Comments from Reviewable

[Artur-]

Review status: 0 of 8 files reviewed at latest revision, 4 unresolved discussions.

---

addon/src/main/java/com/vaadin/board/Row.java, line 195 at r1 (raw file):

            Component comp = it.next();
            Element childElement = designContext.createElement(comp);
            int boarCols = getCols(comp);

boardCols

---

Comments from Reviewable

[rogozinds]

Reviewed 2 of 8 files at r1. Review status: 2 of 8 files reviewed at latest revision, 4 unresolved discussions.

---

addon/src/main/java/com/vaadin/board/Board.java, line 86 at r1 (raw file):

Previously, Artur- (Artur) wrote…

should `Row` be `` or whatever you have in the declarative file?

Done.

---

addon/src/main/java/com/vaadin/board/Row.java, line 180 at r1 (raw file):

Previously, Artur- (Artur) wrote…

Shouldn't this be `:board-cols` to be compatible with the spec that says unprefixed attributes belong to the element they are added to and a ":" prefix indicates they belong to the parent. That also makes the custom `board-` prefix unnecessary so it can be just `:cols`

I think current one makes things just more clear. The issue is that children component can have different board-cols value. For example: it will be

---

addon/src/main/java/com/vaadin/board/declarative/BoardComponentMapper.java, line 10 at r1 (raw file):

Previously, Artur- (Artur) wrote…

What is this class for?

That's acustom component mapper, we need it because the default Component mapper is used only for Vaadin Components, that are in com.vaadin.ui package. So if not using CustomComponent mapper, default one will try to initiate the com.vaadin.ui.Board class? Instead of com.vaadin.board.Board.

---

Comments from Reviewable

[rogozinds]

Review status: 1 of 8 files reviewed at latest revision, 4 unresolved discussions.

---

addon/src/main/java/com/vaadin/board/Row.java, line 195 at r1 (raw file):

Previously, Artur- (Artur) wrote…

boardCols

Done.

---

Comments from Reviewable

[Artur-]

Review status: 1 of 8 files reviewed at latest revision, 2 unresolved discussions.

---

addon/src/main/java/com/vaadin/board/Row.java, line 180 at r1 (raw file):

Previously, rogozinds (rogozinds) wrote…

I think current one makes things just more clear. The issue is that children component can have different board-cols value. For example: it will be

Ok, but the spec says it's wrong

---

addon/src/main/java/com/vaadin/board/declarative/BoardComponentMapper.java, line 10 at r1 (raw file):

Previously, rogozinds (rogozinds) wrote…

That's acustom component mapper, we need it because the default Component mapper is used only for Vaadin Components, that are in com.vaadin.ui package. So if not using CustomComponent mapper, default one will try to initiate the com.vaadin.ui.Board class? Instead of com.vaadin.board.Board.

It's not used anywhere? Also the API is static so you can't really use it from an add-on, it's only for the application developer. The declarative is explicitly that vaadin- maps to com.vaadin.ui.

---

Comments from Reviewable

[rogozinds]

Review status: 1 of 8 files reviewed at latest revision, 2 unresolved discussions.

---

addon/src/main/java/com/vaadin/board/Row.java, line 180 at r1 (raw file):

Previously, Artur- (Artur) wrote…

Ok, but the spec says it's wrong

Fixed

---

addon/src/main/java/com/vaadin/board/declarative/BoardComponentMapper.java, line 10 at r1 (raw file):

anywhere It's used in tests. And in testUI. This is how it's done for charts. You need to set the mapper first.

---

Comments from Reviewable

[rogozinds]

Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion.

---

addon/src/main/java/com/vaadin/board/declarative/BoardComponentMapper.java, line 10 at r1 (raw file):

Previously, rogozinds (rogozinds) wrote…

> anywhere It's used in tests. And in testUI. This is how it's done for charts. You need to set the mapper first.

Removed Custom mapper ,changed tests to use package-mapping

---

Comments from Reviewable

[rogozinds]

Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion.

---

addon/src/main/java/com/vaadin/board/declarative/BoardComponentMapper.java, line 10 at r1 (raw file):

Previously, rogozinds (rogozinds) wrote…

Removed Custom mapper ,changed tests to use package-mapping

Done.

---

Comments from Reviewable

[DiegoCardoso]

---

Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[Artur-]

Reviewed 1 of 8 files at r1, 1 of 2 files at r2, 6 of 6 files at r3. Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

addon/src/test/java/com/vaadin/board/declarative/BoardWriterTest.java, line 19 at r3 (raw file):

    private static final String BOARD_WITH_ONE_ROW_CONFIG=
        "<vaadin-board size-full> \n" +

vaadin-board?

---

addon/src/test/java/com/vaadin/board/declarative/BoardWriterTest.java, line 114 at r3 (raw file):

            String doc = outputStream.toString("UTF-8");

            doc = doc.replace("com_vaadin_board-board","vaadin-board");

This looks like a confusing hack. Why not add package mapping in the declarative?

---

Comments from Reviewable