Skip to content

Description#251

Closed
tinahoa wants to merge 3 commits intothoth-tech:developmentfrom
tinahoa:migrate/viewer-2024t2
Closed

Description#251
tinahoa wants to merge 3 commits intothoth-tech:developmentfrom
tinahoa:migrate/viewer-2024t2

Conversation

@tinahoa
Copy link

@tinahoa tinahoa commented Sep 19, 2024

Migrate the viewer from CoffeeScript to TypeScript

Before

Screenshot 2024-09-19 at 6 35 28 PM ## After Screenshot 2024-09-19 at 12 09 59 PM

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Login to tutor account, under task-dropdown menu, select "Task" button to see all the tasks in a specific unit

Testing Checklist:

  • Tested in latest Chrome
  • Tested in latest Safari
  • Tested in latest Firefox

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@b0ink
Copy link

b0ink commented Sep 19, 2024

Hi @tinahoa

The new route is working well and loads the task-viewer component with the unit and task definition data correctly

One minor issue i noticed is if you refresh the page after the loading the /viewer route, the dropdown menu no longer works and the "selected unit" state is reset

image

You should also delete the viewer.scss and viewer.tpl.html that was together with viewer.coffee as they are no longer being used

Edit: Similar behaviour is also present in the http://localhost:4200/#/edit_profile route, where refreshing the page loses the current selected unit - as it also uses the new TypeScript routing workflow

@tinahoa
Copy link
Author

tinahoa commented Sep 19, 2024

Hi @b0ink, I have removed the old scss and html files. Thank you for reminding me.

@XinHuang1112
Copy link

The migration of viewer component from AngularJS and CoffeeScript to Angular 17 and TypeScript is well-executed.
Good job, Shen Tian.

@aNebula
Copy link

aNebula commented Oct 14, 2024

Hi @tinahoa

The new route is working well and loads the task-viewer component with the unit and task definition data correctly

One minor issue i noticed is if you refresh the page after the loading the /viewer route, the dropdown menu no longer works and the "selected unit" state is reset
image

You should also delete the viewer.scss and viewer.tpl.html that was together with viewer.coffee as they are no longer being used

Edit: Similar behaviour is also present in the http://localhost:4200/#/edit_profile route, where refreshing the page loses the current selected unit - as it also uses the new TypeScript routing workflow

This issue has not been addressed, hence this PR can not be accepted into the code.

@aNebula aNebula closed this Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants