rico-et22 / elektronik-timetable

A modern school timetable frontend for VULCAN Optivum system • Used by 1,000+ students at ZSE Rzeszów, Poland • Open-source project
https://plan-lekcji.zse.rzeszow.pl
GNU Affero General Public License v3.0
17 stars 6 forks source link

Integration with substitutes #21

Closed MrTomek closed 1 year ago

MrTomek commented 1 year ago

Please add an alert or an exclamation mark icon next to a lesson on the timetables of classes, teachers and classrooms.

Data loaded from:

https://elektronik.rzeszow.pl/uploads/zastepstwa/InformacjeOZastepstwach.pdf
or
https://elektronik.rzeszow.pl/api/replacements.json (ready scraper PDF used for TV in the lobby)

MrTomek commented 1 year ago

Example view:

image

rico-et22 commented 1 year ago

In my opinion, it would be better to replace the original lesson with a substitute lesson plus an exclamation mark (something like school e-journals do), rather than just placing a mark on the original lesson that wouldn't provide direct information about the substitute.

https://elektronik.rzeszow.pl/api/replacements.json (ready scraper PDF used for TV in the lobby)

We already use it for the substitutions table 😉

olekbaran commented 1 year ago

I think we can also add a tooltip while hovering on an exclamation mark with some info about original lesson

felpcereti commented 1 year ago

Hello I was doing something similar to that thing you have planned but there's one problem. It's my second time working with a react application so the code looks bad. obraz

My fork: https://github.com/rico-et22/elektronik-timetable/compare/main...felpcereti:elektronik-timetable:main

rico-et22 commented 1 year ago

@felpcereti That way of implementing this is what I actually was thinking of!

I consulted the "exclamation mark" way with other contributors and it would be too clunky and require additional clicks to display the replacement data.

So, we prefer it your way, as we were planning to.

However, I tried launching your fork and it gets me this error: image with the following env variables:

NEXT_PUBLIC_TIMETABLE_BASE_URL=https://zse.rzeszow.pl/plan-lekcji
NEXT_PUBLIC_REPLACEMENTS_API_URL=https://www.elektronik.rzeszow.pl/api/replacements.json

Please fix the code so that it can be used with ZSE Rzeszów's official data sources first. Then try making a pull request

felpcereti commented 1 year ago

I'm using the same env variables. I changed following lines of code https://github.com/felpcereti/elektronik-timetable/commit/56daa8de8215279c2b2ada6adbd91b4d97ac9f2b#diff-e253519adf5ed18c5b9535b50c8f3553796ba50482c17df141c00ac7d3935074L103

day.length -
          1 -
          day
            .slice()
            .reverse()
            .findIndex((dayHour) => dayHour.length > 0)

to

day.findLastIndex((dayHour) => dayHour.length > 0)

It's a newer method which was added in node version 18.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLastIndex#browser_compatibility

If that's a problem I can just revert the change.

felpcereti commented 1 year ago

Also I wonder if you have any other means of communication like discord, telegram or session.

rico-et22 commented 1 year ago

@felpcereti My discord: rico_et22#5855

Oh, I was still using Node 16, upgraded to 18 and the error is gone.

I think you can make a pull request (prepare for a code review), but before, there are two urgent things to fix:

  1. top priority - fix the responsiveness & font size of the table's links as on smaller sizes (eg. 1024px) the rendering breaks. Remove text-sm from here: image and add w-1/2 class to this container, when displaying a teacher's name: image

2.Increase spacing between groups (both table and list)

rico-et22 commented 1 year ago

Done. PR #24 and #25 . @felpcereti thanks!

MrTomek commented 1 year ago

In my opinion, you still need to add this on the teacher's timetable /teacher.

rico-et22 commented 1 year ago

@MrTomek

In my opinion, you still need to add this on the teacher's timetable /teacher.

Teacher pages fixed in #27