Skip to content

fix: allow replacing footer component #21

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

Merged
javier-godoy merged 1 commit into from
Sep 25, 2024
Merged

fix: allow replacing footer component #21

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 17 additions & 9 deletions src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

/**
* Component that allows to create a floating chat button that will open a chat window that can be
Expand All @@ -61,6 +62,7 @@ public class ChatAssistant extends Div {
private static final String CHAT_HEADER_CLASS_NAME = "chat-header";
private Component headerComponent;
private Component footerComponent;
private VerticalLayout footerContainer;
private VirtualList<Message> content = new VirtualList<>();
private List<Message> messages;
private MessageInput messageInput;
Expand Down Expand Up @@ -99,11 +101,13 @@ public ChatAssistant(List<Message> messages) {
whoIsTyping = new Span();
whoIsTyping.setClassName("chat-assistant-who-is-typing");
whoIsTyping.setVisible(false);
VerticalLayout vl = new VerticalLayout(whoIsTyping, messageInput);
vl.setSpacing(false);
vl.setMargin(false);
vl.setPadding(false);
this.setFooterComponent(vl);
footerContainer = new VerticalLayout(whoIsTyping);
footerContainer.setSpacing(false);
footerContainer.setMargin(false);
footerContainer.setPadding(false);
footerContainer.getElement().setAttribute("slot", "footer");
add(footerContainer);
this.setFooterComponent(messageInput);
this.getElement().addEventListener("bot-button-clicked", this::handleClick).addEventData("event.detail");

Icon minimize = VaadinIcon.CHEVRON_DOWN_SMALL.create();
Expand Down Expand Up @@ -282,13 +286,17 @@ public Component getHeaderComponent() {
/**
* Allows changing the footer of the chat window.
*
* @param component to be used as a replacement for the footer
* @param component to be used as a replacement for the footer, it cannot be null
*/
public void setFooterComponent(Component component) {
Objects.requireNonNull(component, "Component cannot not be null");
if (footerComponent==null) {
this.getElement().executeJs("setTimeout(() => this.shadowRoot.querySelector($0).innerHTML = $1)", ".chat-footer", "<slot name='footer'></slot>");
} else {
this.footerContainer.remove(footerComponent);
}
this.footerComponent = component;
this.getElement().executeJs("setTimeout(() => this.shadowRoot.querySelector($0).innerHTML = $1)", ".chat-footer", "<slot name='footer'></slot>");
component.getElement().setAttribute("slot", "footer");
this.add(footerComponent);
footerContainer.add(footerComponent);
Comment on lines +292 to +299
Copy link

@coderabbitai coderabbitai bot Sep 23, 2024

Choose a reason for hiding this comment

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

Approve with suggestion: Revise JavaScript execution logic

The implementation of setFooterComponent is generally good. The null check and the use of footerContainer are appropriate. However, there's a potential issue with the JavaScript execution logic.

The JavaScript to update the shadow DOM is only executed when footerComponent is null. This might lead to inconsistencies when updating an existing footer. Consider moving this JavaScript execution outside the null check:

public void setFooterComponent(Component component) {
    Objects.requireNonNull(component, "Component cannot not be null");
    if (footerComponent != null) {
        this.footerContainer.remove(footerComponent);
    }
    this.footerComponent = component;
    footerContainer.add(footerComponent);
    
    // Execute JavaScript regardless of whether footerComponent was null
    this.getElement().executeJs("setTimeout(() => this.shadowRoot.querySelector($0).innerHTML = $1)", ".chat-footer", "<slot name='footer'></slot>");
}

This ensures that the shadow DOM is always updated when setting a new footer component.

Suggestion: Explicitly remove MessageInput when setting new footer

While the changes improve footer component management, they don't explicitly address the PR objective of removing the MessageInput when a new footer component is set.

Consider modifying the setFooterComponent method to explicitly remove the MessageInput when setting a new footer component:

public void setFooterComponent(Component component) {
    Objects.requireNonNull(component, "Component cannot not be null");
    
    // Remove existing footer component
    if (footerComponent != null) {
        this.footerContainer.remove(footerComponent);
    }
    
    // Explicitly remove MessageInput if it exists and the new component is not MessageInput
    if (messageInput != null && component != messageInput) {
        this.footerContainer.remove(messageInput);
    }
    
    this.footerComponent = component;
    footerContainer.add(footerComponent);
    
    // Execute JavaScript to update shadow DOM
    this.getElement().executeJs("setTimeout(() => this.shadowRoot.querySelector($0).innerHTML = $1)", ".chat-footer", "<slot name='footer'></slot>");
}

This change ensures that the MessageInput is removed when a new footer component is set, directly addressing the PR objective.

}

/**
Expand Down