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

Text exercises: Highlight selected result in result history timeline #9845

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

EneaGore
Copy link
Contributor

@EneaGore EneaGore commented Nov 21, 2024

Checklist

General

Server

No Server Changes

Client

  • Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
  • I strictly followed the client coding and design guidelines.
  • Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
  • I added multiple screenshots/screencasts of my UI changes.

Changes affecting Programming Exercises

No Changes

Motivation and Context

When using the result history timeline on the result view, it is unclear which submission has been selected by the student (apart from the id on the link).

Description

This PR addresses this issue by highlighting the chosen submission. The selected submission is passed on by the text editor component and is checked on the timeline componented whether the highlighted color for the icon is shown. (primary blue to not interfere with green/red/grey)

Also migrates text-editor.component.ts / text-editor.component.ts and result-history.component.ts / result-history.component.ts to using signals

Steps for Testing

Deploy to TS1 to create multiple AI Feedback

Prerequisites:

  • 1 Instructor
  • 1 Student
  • 1 Text Exercise with AI Feedback enabled
  1. Create multiple submissions and request AI Feedback
  2. Check that everything still works
  3. Select one of the last 5 results to view in the timeline (does not work for older)
  4. Confirm that the correct result is highleted in the timeline

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Performance Review

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance even for very large courses with more than 2000 students.
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Screenshots

correctHighlight

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced dynamic behavior in the text editor and result history components with function-based property access.
    • Introduced new input binding for the latest result ID in the result history component.
  • Bug Fixes

    • Improved visibility and state management for UI elements based on dynamic conditions.
  • Tests

    • Updated test cases to align with new input handling methods, ensuring robust validation of component behavior.

@github-actions github-actions bot added the client Pull requests that update TypeScript code. (Added Automatically!) label Nov 21, 2024
@EneaGore EneaGore changed the title highlight selected result ´General´: Hihglight selected result in result history timeline Nov 21, 2024
@EneaGore EneaGore changed the title ´General´: Hihglight selected result in result history timeline General: Hihglight selected result in result history timeline Nov 21, 2024
@EneaGore EneaGore temporarily deployed to artemis-test1.artemis.cit.tum.de November 21, 2024 12:06 — with GitHub Actions Inactive
= Enea_Gore added 2 commits November 21, 2024 13:11
@EneaGore EneaGore temporarily deployed to artemis-test1.artemis.cit.tum.de November 21, 2024 12:41 — with GitHub Actions Inactive
@github-actions github-actions bot added deployment-error Added by deployment workflows if an error occured and removed deploy:artemis-test1 labels Nov 25, 2024
@EneaGore EneaGore removed the deployment-error Added by deployment workflows if an error occured label Nov 25, 2024
@FelixTJDietrich FelixTJDietrich changed the title General: Hihglight selected result in result history timeline General: Highlight selected result in result history timeline Nov 26, 2024
Copy link

github-actions bot commented Dec 6, 2024

⚠️ Unable to deploy to test servers ⚠️

Testserver "artemis-test1.artemis.cit.tum.de" is already in use by PR #9955.

@github-actions github-actions bot added the deployment-error Added by deployment workflows if an error occured label Dec 6, 2024
@EneaGore EneaGore added deploy:artemis-test1 and removed deployment-error Added by deployment workflows if an error occured labels Dec 6, 2024
@EneaGore EneaGore temporarily deployed to artemis-test1.artemis.cit.tum.de December 6, 2024 17:40 — with GitHub Actions Inactive
@EneaGore EneaGore changed the title Text-exercises: Highlight selected result in result history timeline Text exercises: Highlight selected result in result history timeline Dec 6, 2024
Copy link
Contributor

@SimonEntholzer SimonEntholzer left a comment

Choose a reason for hiding this comment

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

Code, and tested on Ts1.

Copy link
Member

@BBesrour BBesrour left a comment

Choose a reason for hiding this comment

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

Tested on TS1, result is correctly highlighted.
There is however a small annoying issue that only the left half of that component is clickable, while the other half isn't.
image

Copy link
Contributor

@magkue magkue left a comment

Choose a reason for hiding this comment

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

Tested on TS1, works as expected

Copy link
Contributor

@coolchock coolchock left a comment

Choose a reason for hiding this comment

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

Tested on TS1, LGTM

Copy link

@sawys777 sawys777 left a comment

Choose a reason for hiding this comment

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

Tested on TS1, everything works fine as expected

Copy link

@HawKhiem HawKhiem left a comment

Choose a reason for hiding this comment

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

Tested on TS1. Everything works as described

image

@EneaGore EneaGore requested a review from a team as a code owner December 22, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) ready for review tests
Projects
Status: Ready For Review
Status: In progress
Development

Successfully merging this pull request may close these issues.

9 participants