vaadin / docs

Official documentation for Vaadin and Hilla.
https://vaadin.com/docs
Other
29 stars 183 forks source link

"Split Layout" page appears to be outdated, uses a different structure from shown in the Java/TS API docs #1417

Open starg09 opened 2 years ago

starg09 commented 2 years ago

So while testing around, I was using the official docs as a guide for how many components work. And in the process of setting up a SplitLayout, I noticed the imports for MasterContent() and DetailContent() weren't showing up. After looking around, it appears these aren't a thing at all? Couldn't find them in the api at least, since v14 (working on v23 myself), it only takes two regular Components, either in constructor or through splitLayout.addToPrimary(first) / splitLayout.addToSecondary(second).

I'm not that familiar with the typescript side of examples, but they'd appear to be mismatching as well.

As a minor example, the initial declarations goes from

MasterContent master = new MasterContent();
DetailContent detail = new DetailContent();

SplitLayout splitLayout = new SplitLayout(master, detail);

to

SplitLayout splitLayout = new SplitLayout();

Should that page be overhauled with examples using the current API? Wouldn't mind forking and sending a PR about it, but would like to confirm I'm not just misunderstanding something 😅

Anyway, thanks for all the work in the documentation, it's been extremely helpful so far!


Some relevant links:

tarekoraby commented 2 years ago

Thanks for the report! MasterContent and DetailContent are regular Vaadin components (here is the MasterContent.java). The constructor new SplitLayout(master, detail) is simply adding master and detail components as the primary and secondary components of the SplitLayout, respectively.

So in a sense, the API usage of the examples is fine. There is still a problem in the examples, however, as it is not clear in the code snippet that MasterContent and DetailContent are just Vaadin components. So I'd say that it might be easier to use a simple component (like a HorizontalLayout) to populate the SplitLayout.

jouni commented 2 years ago

I think we should just include the MasterContent and DetailContent sources in the example directly, so that you can discover that they are extensions of Component. But I suppose we should then also include the sources to the client-side implementations.

tarekoraby commented 2 years ago

But if the point is to demonstrate the SplitLayout API, what is the advantage of including MasterContent and DetailContent compared to just having a simple Div or VerticalLayout?

jouni commented 2 years ago

Making the rendered preview more representative of the use case for which Split Layout is commonly used for. Though, the current implementation of the skeleton screens is not the best, and perhaps making it even more difficult to discern what is going on in the UI.

if the point is to demonstrate the SplitLayout API

The point is not only that. It’s also to illustrate common use cases for which the component is meant for, giving design guidance.

The current documentation is actually produced from that point of view, and the API is secondary, in the sense that we just show how to accomplish the use case being demonstrated.

We wanted to include purely technical documentation as well, but no one has gotten to doing that. Some components have a bit of that, Grid and Upload even dedicated tabs for that, but there hasn't been a concentrated effort in writing examples that focus on the technical features and highlighting some parts of the API.

jouni commented 1 year ago

Related: #1898