Skip to content

Conversation

@sissbruecker
Copy link
Contributor

Updates the Accordion page and examples to use the Aura theme.

Noteworthy changes:

  • Simplified accordion summary example to only show an icon in the accordion heading once the continue button is clicked. The Aura styles do not stretch the heading content, thus the respective panel details were glued to the panel title and it would have required some CSS to make this work. IMO this works out fine, in general the example felt too complex and the Binder setup was also distracting.
  • Updated accordion content example to have colored links with underline
  • Removed small theme variant example as it isn't supported by Aura

@github-actions
Copy link

AI Language Review

AccordionContent.java:

  • Ensure consistent use of color variables. The createStyledAnchor method sets the anchor text color to "var(--aura-blue-text)". Verify if this is the correct variable and if it matches the theme's color standards.

AccordionSummary.java:

  • Ensure color variables like "var(--aura-green-text)" are defined and available in the theme for styling icons consistently.

accordion-styling.adoc:

  • There are duplicate entries in the style variant descriptions, particularly for Filled Panels and Reverse Panels. Ensure that all entries are necessary and not repetitive, as this might confuse the reader.

AccordionExamples.ts:

  • When using string interpolation for complex conditions or operations, such as in ${this.customerComplete ? '...' : '...'}, ensure clarity and simplicity, and consider abstraction for readability if it recurs.

For each of these points, ensure that the styling aspects align with the overarching theme/style guide of the application to maintain consistency in the UI/UX design.

---
= Accordion Styling

== Style Variants
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There probably should be a paragraph about how to apply style variants with a code snippet, especially since the terminology is now different from the theme attribute that you need to use. I think we can separately add a common section for this in a shared asciidoc and then include it in the styling pages that have a style variants section.

>
Continue
</Button>
</VerticalLayout>
Copy link
Member

Choose a reason for hiding this comment

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

Could probably use {/* tag::snippet[] */} and {/* end::snippet[] */} in this example to not include the whole Accordion in the code example, but a single panel like in Lit version. Not a blocker though.

Accordions can be used to break a complex form into smaller step-by-step sections.

The expandable and collapsible nature of accordions can sometimes be difficult for users to discover. Use the <<filled-panels,filled variant>> and apply [since:com.vaadin:vaadin@V23.3]##<<../tooltip#,tooltips>>## on the panels to make this more discoverable.
The expandable and collapsible nature of accordions can sometimes be difficult for users to discover. Use the <<styling#filled-panels,filled variant>> and apply [since:com.vaadin:vaadin@V23.3]##<<../tooltip#,tooltips>>## on the panels to make this more discoverable.
Copy link
Member

Choose a reason for hiding this comment

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

Could probably remove "since" tag from here as it was only relevant for V23 docs.

Copy link
Member

Choose a reason for hiding this comment

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

We should remove all [since] annotations from v25 docs (and any [since v23] annotations from v24 docs).

| Variant | Description | Supported by

|`filled`
|The filled theme variant makes the panel’s boundaries visible
Copy link
Member

Choose a reason for hiding this comment

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

Let's probably align wording here with other variants below:

Suggested change
|The filled theme variant makes the panel’s boundaries visible
|Makes the panel’s boundaries visible


=== Filled Panels

The `filled` theme variant makes the panel's boundaries visible. This helps tie its content together visually and distinguishes it from the surrounding UI.
Copy link
Member

Choose a reason for hiding this comment

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

Let's use "style variant" consistently:

Suggested change
The `filled` theme variant makes the panel's boundaries visible. This helps tie its content together visually and distinguishes it from the surrounding UI.
The `filled` style variant makes the panel's boundaries visible. This helps tie its content together visually and distinguishes it from the surrounding UI.


=== Reverse Panels

The `reverse` theme variant places the toggle icon after the summary contents. This can be useful for aligning visually the summary with other content.
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, let's use "style variant" consistently:

Suggested change
The `reverse` theme variant places the toggle icon after the summary contents. This can be useful for aligning visually the summary with other content.
The `reverse` style variant places the toggle icon after the summary contents. This can be useful for aligning visually the summary with other content.

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.

4 participants