Skip to content

feat: add signal support for Details component summary text #8521

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.

Sign up for GitHub

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

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

feat: add signal support for Details component summary text #8521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0f15e3
feat: add signal support for Details component summary text
platosha Jan 23, 2026
d79d893
Merge branch 'main' into feat/signals-constructor-details
platosha Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 120 additions & 19 deletions ...w-parent/vaadin-details-flow/src/main/java/com/vaadin/flow/component/details/Details.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.vaadin.flow.component.ComponentEventListener;
import com.vaadin.flow.component.HasComponents;
import com.vaadin.flow.component.HasSize;
import com.vaadin.flow.component.SignalPropertySupport;
import com.vaadin.flow.component.Synchronize;
import com.vaadin.flow.component.Tag;
import com.vaadin.flow.component.dependency.JsModule;
Expand All @@ -33,6 +34,7 @@
import com.vaadin.flow.component.shared.HasTooltip;
import com.vaadin.flow.component.shared.SlotUtils;
import com.vaadin.flow.shared.Registration;
import com.vaadin.signals.Signal;

/**
* Details is an expandable panel for showing and hiding content from the user
Expand Down Expand Up @@ -61,6 +63,10 @@ public class Details extends Component implements HasComponents, HasSize,
private final Component summaryContainer;
private final Div contentContainer;

/** Signal support for the summary text property. */
private final SignalPropertySupport<String> summaryTextSupport = SignalPropertySupport
.create(this, this::updateSummaryText);

/**
* Server-side component for the {@code <vaadin-details-summary>} element.
*/
Expand All @@ -81,7 +87,7 @@ public Details() {
SlotUtils.addToSlot(this, "summary", summaryContainer);

if (getElement().getPropertyRaw("opened") == null) {
setOpened(false);
getElement().setProperty("opened", false);
}
Copy link
Contributor

@DiegoCardoso DiegoCardoso Jan 26, 2026

Choose a reason for hiding this comment

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

Is this change needed for the feature to work?


getElement().addPropertyChangeListener("opened", event -> fireEvent(
Expand All @@ -97,7 +103,10 @@ public Details() {
*/
public Details(String summary) {
this();
setSummaryText(summary);
if (summary == null) {
summary = "";
}
Comment on lines +106 to +108
Copy link
Contributor

@DiegoCardoso DiegoCardoso Jan 26, 2026

Choose a reason for hiding this comment

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

This is also done for the other constructor that receives the summary as a String. Does it make sense to move it to the updateSummaryText() method?

updateSummaryText(summary);
}

/**
Expand All @@ -109,7 +118,19 @@ public Details(String summary) {
*/
public Details(Component summary) {
this();
setSummary(summary);
updateSummary(summary);
}

/**
* Initializes a new Details component with a summary text provided by a
* signal.
*
* @param summaryTextSignal
* the signal that provides the summary text
*/
public Details(Signal<String> summaryTextSignal) {
this();
summaryTextSupport.bind(summaryTextSignal);
}

/**
Expand All @@ -125,8 +146,11 @@ public Details(Component summary) {
*/
public Details(String summary, Component content) {
this();
setSummaryText(summary);
add(content);
if (summary == null) {
summary = "";
}
updateSummaryText(summary);
contentContainer.add(content);
}

/**
Expand All @@ -142,8 +166,23 @@ public Details(String summary, Component content) {
*/
public Details(Component summary, Component content) {
this();
setSummary(summary);
add(content);
updateSummary(summary);
contentContainer.add(content);
}

/**
* Initializes a new Details component with a summary text provided by a
* signal and content.
*
* @param summaryTextSignal
* the signal that provides the summary text.
* @param content
* the content component to add.
*/
public Details(Signal<String> summaryTextSignal, Component content) {
this();
summaryTextSupport.bind(summaryTextSignal);
contentContainer.add(content);
}

/**
Expand All @@ -160,7 +199,7 @@ public Details(Component summary, Component content) {
*/
public Details(String summary, Component... components) {
this(summary);
add(components);
contentContainer.add(components);
}

/**
Expand All @@ -177,7 +216,22 @@ public Details(String summary, Component... components) {
*/
public Details(Component summary, Component... components) {
this(summary);
add(components);
contentContainer.add(components);
}

/**
* Initializes a new Details component with a summary text provided by a
* signal and optional content components.
*
* @param summaryTextSignal
* the signal that provides the summary text.
* @param components
* the content components to add.
*/
public Details(Signal<String> summaryTextSignal, Component... components) {
this();
summaryTextSupport.bind(summaryTextSignal);
contentContainer.add(components);
}

/**
Expand All @@ -198,13 +252,7 @@ protected Component createSummaryContainer() {
* any previously set summary
*/
public void setSummary(Component summary) {
summaryContainer.getElement().removeAllChildren();
if (summary == null) {
return;
}

this.summary = summary;
summaryContainer.getElement().appendChild(summary.getElement());
updateSummary(summary);
}

/**
Expand All @@ -218,14 +266,19 @@ public Component getSummary() {
}

/**
* Creates a text wrapper and sets a summary via
* {@link #setSummary(Component)}
* Sets the summary text of the details component.
*
* @param summary
* the summary text to set, or {@code null} for empty text
* @throws com.vaadin.signals.BindingActiveException
* if the summary text is currently bound to a signal
* @see #bindSummaryText(Signal)
*/
public void setSummaryText(String summary) {
if (summary == null) {
summary = "";
}
setSummary(new Span(summary));
summaryTextSupport.set(summary);
}

/**
Expand All @@ -236,6 +289,54 @@ public String getSummaryText() {
return summary == null ? "" : summary.getElement().getText();
}

/**
* Updates the summary component. For internal use during initialization and
* signal updates.
*/
private void updateSummary(Component summary) {
summaryContainer.getElement().removeAllChildren();
if (summary == null) {
return;
}

this.summary = summary;
summaryContainer.getElement().appendChild(summary.getElement());
}

/**
* Updates the summary text when bound to a signal.
*/
private void updateSummaryText(String newText) {
if (summary == null || !(summary instanceof Span)) {
updateSummary(new Span(newText));
} else {
summary.getElement().setText(newText);
Copy link
Contributor

@DiegoCardoso DiegoCardoso Jan 26, 2026

Choose a reason for hiding this comment

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

This is not covered by any tests.

}
}

/**
* Binds a signal to update the summary text of the details component.
Copy link
Member

@tltv tltv Jan 26, 2026

Choose a reason for hiding this comment

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

For consistency public bind* methods should say same message all over. For this case HasText::bindText(Signal<String>) works as a good template. Example is not needed since there are similar examples in most Flow API.

* <p>
* When bound, the signal manages the summary text. Any previously set
* custom summary component will be replaced when the signal updates.
*
* @param signal
* the signal that provides the new summary text, or {@code null}
* to remove the binding.
*/
public void bindSummaryText(Signal<String> signal) {
summaryTextSupport.bind(signal);
}

/**
* Gets the summary text signal support instance for testing purposes.
*
* @return the summary text signal support
*/
SignalPropertySupport<String> getSummaryTextSupport() {
return summaryTextSupport;
}

/**
* Adds components to the content section
*
Expand Down
Loading