Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Documentation for each component #12

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Type-32
Copy link

@Type-32 Type-32 commented Mar 22, 2024

Still needs review before merge to check every sample in the component pages is correct.
No more stuff needs to be added.

@Type-32 Type-32 changed the title Added Documentation for each component Draft: Added Documentation for each component Mar 22, 2024
@Type-32
Copy link
Author

Type-32 commented Mar 22, 2024

Added a section with sample code that documents template usage.

@Noaaan
Copy link
Member

Noaaan commented Apr 15, 2024

Might be worth superseding #11 by adding the following into the templates section: https://github.com/wisp-forest/docs/pull/11/files#diff-72a989623bbc727392a2a48635053d40cc954f43eaab0c4429a8b9d615740cb2R7-R27

@Type-32
Copy link
Author

Type-32 commented Jul 15, 2024

New documentation page added with examples and usage, as well as syncing from master branch

@Noaaan
Copy link
Member

Noaaan commented Sep 2, 2024

Is this still a draft?

@Type-32
Copy link
Author

Type-32 commented Sep 3, 2024

I did merge your templates page changes into the commit, and I don't have much to write more on the components docs, so, no

@Noaaan Noaaan changed the title Draft: Added Documentation for each component Added Documentation for each component Sep 3, 2024
@Noaaan Noaaan requested a review from gliscowo September 3, 2024 05:56
@Noaaan Noaaan self-requested a review November 20, 2024 08:10
Copy link
Member

@Noaaan Noaaan left a comment

Choose a reason for hiding this comment

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

Reviewed all the code-specific examples and tested them. Sorry for taking so long, it has been a busy year all-round.

The components have changed in the meantime, and do have small differences. I have gone ahead and provided some updated examples from testing them.

Copy link
Author

@Type-32 Type-32 left a comment

Choose a reason for hiding this comment

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

epic work!

@Noaaan
Copy link
Member

Noaaan commented Mar 10, 2025

Can confirm that all code components are correct. Going to give the data-driven ones a try.

Copy link
Member

@Noaaan Noaaan left a comment

Choose a reason for hiding this comment

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

Overall very good! This batch of testing did uncover some more issues, but none of them are directly caused by the documentation.


```xml
<dropdown>
<close-when-not-hovered>true</close-when-not-hovered>
Copy link
Member

Choose a reason for hiding this comment

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

An important note for this field is that if it is true, then the component will instantly disappear.
For this example I suggest setting the default to false, with a sidenote that the component will instantly close/disappear should the value be true

Copy link
Member

Choose a reason for hiding this comment

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

Elements in a grid require to specify column and row, like so:

 <children>
    <label column="0" row="0">
        <text>Cell 1</text>
    </label>
    <label column="0" row="1">
        <text>Cell 2</text>
    </label>
    <label column="1" row="0">
        <text>Cell 3</text>
    </label>
    <label column="1" row="1">
        <text>Cell 4</text>
    </label>
</children>

This is not present in the schema

Copy link
Member

Choose a reason for hiding this comment

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

It seems like a scroll container does work correctly when created from XML at the moment. This is not an issue with the code-driven approach.

Copy link
Member

Choose a reason for hiding this comment

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

After looking further into the schema, it seems the scroll tag simply accepts any component as its single child. It is defined as so:

    <xs:complexType name="owo-ui-scroll-container">
        <xs:complexContent>
            <xs:extension base="componentType">
                <xs:sequence>
                    <!-- Any component !-->
                    <xs:group ref="anyComponent"/>
                    <!-- -->
                    <xs:choice maxOccurs="unbounded">
                        <xs:group ref="parentComponentProps"/>
                        <xs:element type="xs:unsignedInt" name="scrollbar-thiccness" minOccurs="0"/>
                        <xs:element type="xs:unsignedInt" name="fixed-scrollbar-length" minOccurs="0"/>
                        <xs:element type="owo-ui-color" name="scrollbar-color" minOccurs="0"/>
                        <xs:element type="owo-ui-scrollbar" name="scrollbar" minOccurs="0"/>
                    </xs:choice>
                </xs:sequence>
                <xs:attribute name="direction" type="owo-ui-axis-direction" use="required"/>
            </xs:extension>
        </xs:complexContent>
    </xs:complexType>

As such, a working example looks something like this:

<scroll direction="vertical">
    <!-- Any component as child. We recommend a flow layout-->
    <flow-layout direction="vertical">
        <children>
            <label>
                <text>Item 1</text>
            </label>
            <label>
                <text>Item 2</text>
            </label>
            <label>
                <text>Item 3</text>
            </label>
        </children>
        <sizing>
            <vertical method="fill">20</vertical>
            <horizontal method="fill">25</horizontal>
        </sizing>
    </flow-layout>
    <scrollbar-thiccness>10</scrollbar-thiccness>
    <scrollbar-color>#ff0000</scrollbar-color>
    <padding>
        <all>10</all>
    </padding>
    <surface>
        <panel/>
    </surface>
    <sizing>
        <vertical method="fill">50</vertical>
        <horizontal method="fill">50</horizontal>
    </sizing>
</scroll>

Copy link
Member

Choose a reason for hiding this comment

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

The Slider component should always specify sizing, as by default it is "Content" on both the horizontal and vertical axis. This is a problem, since the slider does not allow content sizing on the horizontal axis.

Example:

      <slider>
              <text>Volume</text>
              <value>0.5</value>
              <active>true</active>
              <sizing>
                  <horizontal method="fill">20</horizontal>
                  <vertical method="fixed">10</vertical>
              </sizing>

I am not sure, but hopefully this could be enforced with the schema?

@Noaaan Noaaan self-assigned this Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants