WIP: API review changes

[ahie]

This change is 

[hesara]

Reviewed 10 of 11 files at r1.
Review status: 9 of 10 files reviewed at latest revision, 3 unresolved discussions.

---

client/src/main/java/com/vaadin/client/renderers/HierarchyRenderer.java, line 97 at r1 (raw file):

    /**
     * Set the style name prefix for the node, expander and cell-content
     * elements.

could perhaps mention that this only changes the settings, not actual active style names

---

server/src/main/java/com/vaadin/data/provider/InMemoryHierarchicalDataProvider.java, line 54 at r1 (raw file):

     * Constructs a new InMemoryHierarchicalDataProvider.
     * <p>
     * All changes made to the given HierarchyData object will also be visible

Shouldn't this still be expressed in some way (also telling what are the limitations)?

---

shared/src/main/java/com/vaadin/shared/ui/composite/CompositeState.java, line 21 at r1 (raw file):

/**
 * Shared state for Composite.

should perhaps mention that this isn't really used on the client side, but rather CompositeConnector.getState() does some tricks to show the composition root component state

---

Comments from Reviewable

[hesara]

Reviewed 6 of 6 files at r2, 4 of 4 files at r3, 2 of 2 files at r4.
Review status: 18 of 19 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[hesara]

Reviewed 1 of 1 files at r5, 1 of 1 files at r6.
Review status: 20 of 21 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

server/src/main/java/com/vaadin/ui/Grid.java, line 3942 at r6 (raw file):

     * @param providers
     *            list of {@link DeclarativeValueProvider}s to store the data of
     *            each column to

@SInCE as AFAIK these were exposed in 8.1

---

server/src/main/java/com/vaadin/ui/Grid.java, line 4002 at r6 (raw file):

     *            data to
     * @param designContext
     *            the design context

@SInCE as above

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 293 at r5 (raw file):

            throw new IllegalArgumentException("No column found for given id");
        }
        getColumn(id).setHidden(false);

(it might be good for this to be implemented by calling the method above to reduce duplication - not a blocker)

---

Comments from Reviewable

[hesara]

Review status: 19 of 21 files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 95 at r7 (raw file):

    /**
     * Creates a new {@code TreeGrid} using the given
     * {@code HierarchicalDataProvider}.

Does the comment from the no-args constructor apply here as well?

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 106 at r7 (raw file):

    /**
     * Creates a {@code TreeGrid} using the given in-memory data.

Does the comment from the no-args constructor apply here as well?

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 119 at r7 (raw file):

    /**
     * Creates a grid using a custom {@link PropertySet} implementation and

Creates a TreeGrid

---

Comments from Reviewable

[hesara]

Reviewed 1 of 7 files at r2, 6 of 6 files at r8, 2 of 2 files at r11.
Review status: 19 of 25 files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

client/src/main/java/com/vaadin/client/connectors/LocalDateRendererConnector.java, line 18 at r9 (raw file):

package com.vaadin.client.connectors;

import com.vaadin.client.connectors.grid.TextRendererConnector;

TRC of 8.0 is now in a different package, but we can't really move it. How many of the renderer connectors are in each of the locations?

---

Comments from Reviewable

[hesara]

Reviewed 2 of 5 files at r12, 3 of 4 files at r14.
Review status: 22 of 31 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

---

server/src/main/java/com/vaadin/ui/ItemCollapseAllowedProvider.java, line 34 at r14 (raw file):

 */
@FunctionalInterface
public interface ItemCollapseAllowedProvider<T> extends SerializablePredicate<T> {

(related changes might conflict with #9318)

---

Comments from Reviewable

[hesara]

Reviewed 2 of 5 files at r12.
Review status: 24 of 31 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

---

client/src/main/java/com/vaadin/client/connectors/LocalDateRendererConnector.java, line 18 at r9 (raw file):

Previously, hesara (Henri Sara) wrote…

TRC of 8.0 is now in a different package, but we can't really move it. How many of the renderer connectors are in each of the locations?

These seem to be a real mess for the old and new renderer connectors, and also e.g. ComponentRendererConnector is a part of the picture. Also, there are base two different base classes for the connectors which probably must not be moved as other renderers outside the framework may inherit them.

Should probably really move all the concrete renderer connectors (or at least all the new ones) to a new package.

---

client/src/main/java/com/vaadin/client/widgets/Grid.java, line 6230 at r14 (raw file):

    }

    private String getFocusPrimaryStyleName() {

(could also eliminate this now)

---

Comments from Reviewable

[hesara]

Reviewed 2 of 2 files at r10.
Review status: 26 of 31 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

---

server/src/main/java/com/vaadin/data/provider/AbstractBackEndHierarchicalDataProvider.java, line 43 at r14 (raw file):

    private List<QuerySortOrder> sortOrders = new ArrayList<>();

    private HierarchicalQuery<T, F> mixInSortOrders(

(should have some comment or javadoc as what this is doing is not immediately obvious (concatenate non-conflicting own sort orders to the query sort orders?)

---

Comments from Reviewable

[pleku]

Review status: 23 of 32 files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

---

server/src/main/java/com/vaadin/data/provider/InMemoryHierarchicalDataProvider.java, line 31 at r17 (raw file):

• A {@link DataProvider} for in-memory hierarchical data.

The javadoc states absolutely nothing about why or when this class should be used, and the why is not even that clear

---

Comments from Reviewable

[ahie]

Review status: 17 of 33 files reviewed at latest revision, 14 unresolved discussions.

---

client/src/main/java/com/vaadin/client/widgets/Grid.java, line 6230 at r14 (raw file):

Previously, hesara (Henri Sara) wrote…

(could also eliminate this now)

Done

---

server/src/main/java/com/vaadin/ui/Grid.java, line 3942 at r6 (raw file):

Previously, hesara (Henri Sara) wrote…

@SInCE as AFAIK these were exposed in 8.1

Done.

---

server/src/main/java/com/vaadin/ui/Grid.java, line 4002 at r6 (raw file):

Previously, hesara (Henri Sara) wrote…

@SInCE as above

Done.

---

server/src/main/java/com/vaadin/ui/ItemCollapseAllowedProvider.java, line 34 at r14 (raw file):

Previously, hesara (Henri Sara) wrote…

(related changes might conflict with #9318)

#9318 changes merged to this PR

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 293 at r5 (raw file):

Previously, hesara (Henri Sara) wrote…

(it might be good for this to be implemented by calling the method above to reduce duplication - not a blocker)

Done.

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 95 at r7 (raw file):

Previously, hesara (Henri Sara) wrote…

Does the comment from the no-args constructor apply here as well?

Done.

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 106 at r7 (raw file):

Previously, hesara (Henri Sara) wrote…

Does the comment from the no-args constructor apply here as well?

Done.

---

server/src/main/java/com/vaadin/ui/TreeGrid.java, line 119 at r7 (raw file):

Previously, hesara (Henri Sara) wrote…

Creates a TreeGrid

Done.

---

Comments from Reviewable

[hesara]

Superceded by #9326, #9327 and a later change (to be made) to move renderer connector classes.