Skip to content

Conversation

ummjevel
Copy link
Contributor

Resolve #272

Purpose

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[ ] New feature
[x] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

README updated?

The top-level readme for this repo contains a link to each sample in the repo. If you're adding a new sample did you update the readme?

[ ] Yes
[x] No
[ ] N/A

How to Test

  • Get the code
git clone https://github.com/aliencube/open-chat-playground.git
cd open-chat-playground
git checkout feature/272-uicomprefactor-chatmsglist
  • Test the code
dotnet build
dotnet test --filter "FullyQualifiedName~ChatMessageListUITests"

What to Check

Verify that the following are valid

  • Component separation: UI (.razor) and logic (.razor.cs) properly separated
  • Test coverage: 9 UI tests passing with Theory/InlineData pattern

Other Information

  • Refactoring scope: Single .razor file → separated structure following code-behind pattern
  • Test strategy: E2E tests maintained, LLM dependencies removed, focused on component rendering only

public partial class ChatMessageList : ComponentBase
{
[Inject]
private IJSRuntime JS { get; set; } = default!;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 여기도 public으로 바꿔 주세요.
  • public IJSRuntime? JS { get; set; } 으로만 하면 어떤가요?

{
if (firstRender)
{
await JS.InvokeVoidAsync("import", "./Components/Pages/Chat/ChatMessageList.razor.js");
Copy link
Contributor

Choose a reason for hiding this comment

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

그러면 여기를 JS!.InvokeVoidAsync(...)로 바꾸면 될 듯?

Comment on lines +54 to +58
// Act - Check initial state before any user interaction
var chatMessages = Page.Locator("chat-messages");
var inProgressAttribute = await chatMessages.GetAttributeAsync("in-progress");

// Assert - Initially no InProgressMessage, so attribute should be null
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 보면 // Act 뒤에, // Assert 뒤에 설명이 붙어 있는데, 이 설명이 없으면 이 테스트 메소드가 설명이 안 되는 건가요? 만약 그렇다면 테스트 메소드 이름에 저 설명을 녹여보면 어떨까요?

Comment on lines +75 to +79
// Verify container structure remains intact
var containerExists = await messageContainer.IsVisibleAsync();
var inputValue = await textArea.InputValueAsync();

// Assert - Verify ChatMessageList container is functional and input works
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도 마찬가지

Comment on lines +104 to +121
// Act - Test the empty state rendering of ChatMessageList
if (expectedElement == "To get started, try asking about anything.")
{
var noMessagesElement = Page.Locator(".no-messages");
var isVisible = await noMessagesElement.IsVisibleAsync();
var content = await noMessagesElement.InnerTextAsync();

// Assert - Verify NoMessagesContent is properly rendered
isVisible.ShouldBeTrue();
content.ShouldContain(expectedElement);
}
else if (expectedElement == "User Message Textarea")
{
// Test input field exists
var textArea = Page.GetByRole(AriaRole.Textbox, new() { Name = expectedElement });
var inputExists = await textArea.IsVisibleAsync();
inputExists.ShouldBeTrue();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

전에도 말했지만, 테스트 메소드 안에 if ... else 구문이 있으면 테스트 코드를 잘 못 짠 것입니다. 각각의 조건에 맞춰서 테스트 메소드를 새로 만드세요.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI Component Refactoring: ChatMessageList.razor

2 participants