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

Redesign of / page #964

Merged
merged 70 commits into from
Apr 28, 2023
Merged

Redesign of / page #964

merged 70 commits into from
Apr 28, 2023

Conversation

MatthiasReumann
Copy link
Collaborator

@MatthiasReumann MatthiasReumann commented Mar 28, 2023

Motivation and Context

Redesign and reimplement the start page using best practices of Alpine, Tailwind, and TypeScript.

Description

  • New UI
  • 99% REST. Little to none go-templates (no more x-data="json.stringify({{data}})"!)
  • TUM-Live CSS classes
  • Proposing updated TypeScript folder structure:
    • views/: Whole pages (used only once)
    • components/: Single UI Elements (used more than once, e.g. a header module)
    • utilities/: Reusable Classes, Functions, Enums, etc. that can be included in components and views
    • api/: API-Wrappers, DTOs, objects
    • entry/: Combination of views and components to bundle functionality as close as possible
  • Add tools/pathprovider/pathprovider.go to reduce code duplication and a place to put present and future file-path stuff.

Help needed 🙋‍♂️

There is probably a better way to built multiple css files than my current approach. I simply appended another npx command to the existing one.

webpack --config webpack.dev.js & npx tailwindcss build -i assets/css/main.css -o ./assets/css-dist/main.css --watch & npx tailwindcss build -i assets/css/home.css -o ./assets/css-dist/home.css --watch

❓Are there any changes needed in the production config (webpack)?

Potential follow-ups

I propose the following Follow-Ups in order to decrease the size of this PR:

  • Custom thumbnail upload
  • Move API objects (Bookmarks, VideoSections) to separate file in api/.
    • Currently they reside in data-store/
    • See: api/ for examples

Steps for Testing

Head to / and click on everything you can in all the browsers you have installed.

case "IRH102":
return "https://nav.tum.de/room/5620.01.102";
default:
return "#";
Copy link
Member

Choose a reason for hiding this comment

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

I think this could be a better fallback (with importing fmt):

Suggested change
return "#";
return fmt.Sprintf("https://nav.tum.de/search?q=%s", this.Name);

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

IMO we should add a new field to the lecture_halls table, e.g. string ExternalLink instead of mapping them in the codebase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. Should we add the column in this PR or in a follow-up?

Copy link
Collaborator Author

@MatthiasReumann MatthiasReumann Apr 24, 2023

Choose a reason for hiding this comment

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

Did it in this one. Ids don't necessarily have to be the same, I guess.

Here's the SQL insert statement:
update lecture_halls
set external_url = 'https://nav.tum.de/room/5602.EG.001'
where id = 1;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5604.EG.011'
where id = 2;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5606.EG.011'
where id = 3;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5620.01.101'
where id = 4;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5620.01.102'
where id = 5;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5608.EG.038'
where id = 6;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5613.EG.009A'
where id = 7;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5607.EG.014'
where id = 9;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5510.EG.001'
where id = 10;
update lecture_halls
set external_url = 'https://nav.tum.de/room/5510.02.001'
where id = 11;

@CommanderStorm
Copy link
Member

CommanderStorm commented Apr 18, 2023

Some nitpicks:

  • when no semeser is selected, this part of the UI looks a bit broken, I think selecting the most recent one by default is a save bet:
    image
  • About, Data Privacy, Imprint and We ❤️ OpenSource lead to an empty page
  • These scroll bars are kind of not necessary, but no user will notice given that tum.live has a lot more content in prod
    image

Thanks for the feedback! 💙

1 and 2 are test server related. The about, imprint, data privacy pages aren't stored in the database. The current semester is actually automatically selected if no Year/Term is specified, however, SS23 isn't in the database either 😅

The scroll bars are unfortunately only an issue on certain operating systems. For example, on MacOS the start page looks like:
image

I will style the scroll bar to be invisible. Thanks for the catch!

Copy link
Member

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

I like this change (I am not qualified, this is why this is ony a comment not an aproval)
I have added some inline comments, but nothing major

api/courses.go Show resolved Hide resolved
web/assets/css/home.css Show resolved Hide resolved
web/assets/css/home.css Outdated Show resolved Hide resolved
web/assets/init.js Outdated Show resolved Hide resolved
web/assets/init.js Outdated Show resolved Hide resolved
web/assets/init.js Outdated Show resolved Hide resolved
web/assets/init.js Outdated Show resolved Hide resolved
web/template/home.gohtml Outdated Show resolved Hide resolved
web/template/home.gohtml Show resolved Hide resolved
web/template/home.gohtml Outdated Show resolved Hide resolved
@MatthiasReumann
Copy link
Collaborator Author

MatthiasReumann commented Apr 24, 2023

Update 24.4.2023

  • Server Notifications
  • Pagination for recent VODs to reduce massive one time server load (e.g. ~100 thumbnails) and improve responsiveness
  • Add ExternalURL to lecture hall model

image

Copy link
Sponsor Member

@joschahenningsen joschahenningsen left a comment

Choose a reason for hiding this comment

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

Amazing, looking forward to seeing this on prod :)

@joschahenningsen joschahenningsen merged commit 163adc1 into dev Apr 28, 2023
@joschahenningsen joschahenningsen deleted the enh/start-page branch April 28, 2023 11:23
SebiWrn pushed a commit that referenced this pull request May 7, 2024
* Add temporary home route

* minor changes

* Update dropdown

* Add basic structure

* Minor ui changes

* Add semesters api endpoint

* Add current semester to api

* Add livestream endpoint

* Add users endpoint

* Add my-courses view

* Introduce views in ts

* Clean up fetch function

* Update public course api

* Add live-today

* Add recent-vod list

* Update today and recent lecture ts

* Add progress to start page videos

* Add query params

* Add ts routing

* Clean up typescript

* Update grid-cols

* Merge endpoints

* Add batch progress endpoint

* Make browser undo work with js

* Add data classes

* Fix undo bug

* Update Data classes

* Add css classes

* Update css

* Add view to history

* Test config

* Fix un-logged-in errors

* Remove old start page

* Add init.js

* Add nothing to do

* Make notifications usable mobile

* Add comments; Remove old index page files

* Update ToggleableElement

* Remove notifications from old header

* update side navigation css

* Remove unused function

* Update TS directory structure

* Add live thumbnails

* Add correct navigtum urls

* Add ts/api folder

- Slim down home.ts

* Add comment

* Clean up handler

* Add permissions for thumbnails

* Hide scrollbars

* Forgot the notification scroll bar ^

* Format css file

* Add return after error

* Remove unused variables

* Clean up init.js

* Remove icon code

* Inline function

* add dtos, fix loading of courses for lecturers

* Revert "Remove old start page"

This reverts commit 091529e.

* restore old start page

* use /new for new start page

* allow thumbnails for vod

* use gin.File for live thumbs

* slightly modify video sizes and fix aspect ratio to 16/9

* Add server notifications

* Add paginator

* Add external_url to lecture hall model

---------

Co-authored-by: Joscha Henningsen <joscha.henningsen@tum.de>
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.

Live now vs Live Start page redesign
4 participants