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

series.js: remove jquery and convert to typescript #4266

Merged
merged 4 commits into from
Feb 2, 2023

Conversation

freyavs
Copy link
Contributor

@freyavs freyavs commented Dec 25, 2022

This pull request removes all jquery from series.js and converts it to typescript. It also fixes some small bugs.

Fix 1: a small bug for the css on activities: before, when adding pending and new to an activity tr-element, this didn't have any effect on the css. Now when adding pending or new, the opacity is correctly changed (in the short time frames it takes to add an activity to a series):

Before (when pending):
2023-01-04_10-58
After (when pending):
2023-01-04_10-57

Fix 2: I also made sure that a user can't add/remove the same activity multiple times. This was possible when it was taking a longer time for adding/removing an activity (e.g. waiting on a response), so these changes are only visible for this edge case. For adding, the add button was not hidden when a series is being added (in the new and pending stages). The add button only became hidden when the series was successfully added. This made it possible to add multiple instances of the same activity. The new flow is

  • the add button gets hidden directly, and becomes visible again on addingActivityFailed
  • For removing, the remove button gets hidden (see image below), and becomes visible on removingActivityFailed.

Before (when pending):
2023-01-04_11-33

After (when pending):
2023-01-04_11-38

This is progress on #3590

@freyavs freyavs added the chore Repository/build/dependency maintenance label Dec 25, 2022
@freyavs freyavs self-assigned this Dec 25, 2022
@freyavs freyavs mentioned this pull request Dec 25, 2022
9 tasks
@freyavs freyavs marked this pull request as ready for review January 4, 2023 11:08
@freyavs freyavs requested a review from a team as a code owner January 4, 2023 11:08
@freyavs freyavs requested review from jorg-vr and chvp and removed request for a team January 4, 2023 11:08
Copy link
Contributor

@jorg-vr jorg-vr left a comment

Choose a reason for hiding this comment

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

Code looks good :) Thanks for the bug fixes!

Just found one small extra improvement, which we might as well include while we're busy refactoring this code

app/javascript/packs/evaluation.js Outdated Show resolved Hide resolved
app/javascript/packs/series.js Outdated Show resolved Hide resolved
@bmesuere
Copy link
Member

bmesuere commented Jan 6, 2023

(I would wait to merge this one till after the exams)

@jorg-vr jorg-vr merged commit 80860db into develop Feb 2, 2023
@jorg-vr jorg-vr deleted the chore/series-jquery-removal branch February 2, 2023 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Repository/build/dependency maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants