feat: add location to "Scheduled" lectures on course page #1376
Conversation
|
Your Testserver will be ready at https://1376.test.live.mm.rbg.tum.de in a few minutes. Logins
|
KemalKrKX
requested review from
joschahenningsen,
SebiWrn and
DawinYurtseven
web/template/home.gohtml
Outdated
| @@ -567,7 +567,9 @@ | |||
| </section> | |||
| </template> | |||
| <template x-if="plannedStreams.hasElements()"> | |||
| <section class="tum-live-course-view-item px-3"> | |||
| <section x-data="{isLectureHallValid(lectureHall) {const regex = /^\d{4}\.[A-Z0-9]{2}\.[A-Z0-9]{3,4}$/; | |||
There was a problem hiding this comment.
Please move this function into the corresponding typescript file. Especially functions shouldn't be in the x-data property as this should only contain data.
There was a problem hiding this comment.
I created a lecture-hall-validator.ts in the /web/ts/utilities, but I am having problems determining where Alpinejs is initialized so that i can import and use this validator. Do you happen to know where it is?
KemalKrKX
force-pushed
the
1193-lectureHalls
branch
from
5c11f0b
to
cd7b8dc
Compare
| return regex.test(lectureHall); | ||
| } | ||
|
|
||
| declare global { |
There was a problem hiding this comment.
Why do we need this declare global here? Wouldn't it be enough to just export the function and use it?
There was a problem hiding this comment.
Theoretically it should suffice, but my local copy would not use this function unless I made it global. I can try making it work without the global declaration if it is an issue.
There was a problem hiding this comment.
We shouldn't have unnecessary global definitions so it would be better to change it
CommanderStorm
changed the title
|
Couldn't get the validator from a separate utility file to work with the website template even after testing various methods. Moved the script inside the home.gohtml instead. Hope this is alright. |
|
I'm sorry but we just have to put that code into the .ts files. |
|
I am not entirely sure what the exact cause is, but when I export the function to a seperate file the check breaks. A global definition solves this, so i believe the file/function is out of scope, however every bypass i could think until now caused further problems. |
web/template/home.gohtml
Outdated
| @@ -566,6 +567,7 @@ | |||
| </section> | |||
| </section> | |||
| </template> | |||
| <div x-data="{ isLectureHallValid }"> | |||
There was a problem hiding this comment.
Why is this data Tag necessary? I think you don't need it.
There was a problem hiding this comment.
Yeah, it is part of another change that i scrapped and it seems i forgot to delete it later on.
There was a problem hiding this comment.
But now you again included these lines.
There was a problem hiding this comment.
Sorry, I forgot I changed these lines on Github directly and when I pushed my local it must have overridden the change.
|
Is the update of the package-lock.json really needed? If not, we should do this in a different PR so we can locate errors in the git log more easily. |
|
This is a byproduct of me using |
|
I think we can rollback these changes and if we need we can do a full upgrade of every dependency in our project in another PR sometime. |
KemalKrKX
force-pushed
the
1193-lectureHalls
branch
from
86e502a
to
1ecf12d
Compare
|
The |
web/template/home.gohtml
Outdated
| @@ -566,6 +567,7 @@ | |||
| </section> | |||
| </section> | |||
| </template> | |||
| <div x-data="{ isLectureHallValid }"> | |||
There was a problem hiding this comment.
But now you again included these lines.
Motivation and Context
Under scheduled lectures a new link to lecture hall webpage on NavigaTUM is added.
The issue: #1193
Description
The lecture hall information of scheduled lectures is used to query NavigaTUM.
Steps for Testing
Prerequisites:
Local copies of TUMLive don't update the lecture hall information, so no link is displayed there. However a reviewer can manually set a RoomCode for the lecture in order to observe the change.