[Enhancement] Convey more structure in the DOM tree code.

mk-pmb commented 2 years ago

Is your enhancement request related to a problem? Please describe.

My browser injects a custom style sheet for me. I'd like to use that to re-style the related videos sidebar into a gallery view with multiple cards per row. This is harder than it should be:

Lots of the containers have no descriptive class= or id=, so I have to use #player-container ~ .pure-g and couting selectors a lot.
The sidebar entries each are a pair of a+h5 on the same level.

Describe the solution you'd like

Let's name more stuff in the DOM.
Let's wrap sidebar entries in a div.

Describe alternatives you've considered

Have a bookmarklet transform the code.

Additional context

Additional search keywords

Semantic HTML: Is something else and I probably won't achieve it in this effort, but it's probably a step towards that.

mk-pmb commented 2 years ago

Another obstacle is that all the #player-container ~ .pure-g > :last-child > .h-box > a div.thumbnail + p ¹ carry their own style="width:100%" attribute. Once we have descriptive CSS classes, let's please move that direct styling to the style sheet.

¹) In the future, #player-container ~ .pure-g > :last-child might become #related-videos-box and their a div.thumbnail + p might become .video-title. Then this long, cryptic syntax could become just #related-videos-box .video-title.

SamantazFox commented 2 years ago

Hello! thanks for your feedback :)

This is indeed planned! I'm trying to push class names and ids when UI components are rewritten. I've also been trying to remove as much inline styles as possible.

It takes quite some time because there are so many things to be fixed, between the crystal code, the JS for the player and the many HTML pages, and we're only a few people working on invidious.

If you'd like to help us doing that, I'd be happy to guide you through the code :)

mk-pmb commented 2 years ago

Do we have to fix all at once, or could we introduce the class names progressively? What kind of rewrite is planned for the UI components?

SamantazFox commented 2 years ago

I think it's better if we implement them progressively. Makes changes easier to review, test and rollback if that broke something :)

Regarding the work in progress:

Search filters: #2544.
Channel pages (videos, channel playlists, related channels, etc...): #2546
Footer: #2215
User pages: #2377 plus another PR I haven't pushed yet (basically preferences + import/export + account management pages (password, tokens, subscriptions))

Also note that at the moment, the light/dark theme switching is based on CSS media queries, and that color definitions are almost always duplicated. For instance, links style is done like so:

.light-theme a:hover,
.light-theme a:active,
.light-theme summary:hover {
  color: #075A9E !important;
}

@media (prefers-color-scheme: light) {
  .no-theme a:hover,
  .no-theme a:active,
  .no-theme summary:hover  {
    color: #075A9E !important;
  }
}

.dark-theme a:hover,
.dark-theme a:active,
.dark-theme summary:hover {
  color: rgb(0, 182, 240);
}

@media (prefers-color-scheme: dark) {
  .no-theme a:hover,
  .no-theme a:active {
    color: rgb(0, 182, 240);
  }
}

I know that this is far from optimal and should be cleaned by using <property>: var(--variable); with the corresponding --variable declaration in the html element CSS:

html.light-theme {
  --link-color: #075A9E;
}

@media (prefers-color-scheme: light) {
  html.no-theme  {
    --link-color: #075A9E;
  }
}

html.dark-theme {
  --link-color: rgb(0, 182, 240);
}

@media (prefers-color-scheme: dark) {
  html.no-theme {
    --link-color: rgb(0, 182, 240);
  }
}

a:hover, a:active, summary:hover  {
  color: var(--link-color);
}

mk-pmb commented 1 year ago

Somehow I totally missed your reply; I only stumbled upon this thread again when I was going to request this feature and then found I had already done so. :D

Most of the issues linked seem closed now. What's the current best way to help? Should I just fire up a Docker instance, try tracing the offending DOM back to its templates and patch them?

SamantazFox commented 1 year ago

It happens ^^

Yeah, quite a few things have changed since then!

A docker is probably the easiest way to make change and try them out (clone repo, make changes, run docker-compose up and you're good to go).

However, due to the compiled nature of the Crystal language, changes to the .cr an .ecr files will require a compilation step which may be a bit lengthy on some hardware. If you plan to do a lot of small changes, either start with your browser devtools, or don't use docker and compile with make RELEASE=0 so that compiled objects will be reused, which reduce compilation time!

Code locations

ECR Templates

These templates are mostly HTML with some crystal in it. They're located here: https://github.com/iv-org/invidious/tree/2414e7db411f9de7ba40cad370d4b195830d4456/src/invidious/views

template.ecr is the base skeleton of all pages (except for /embed). All other views depends on that.

Views are generally named after the page they're used for. Look for templated in the code to see where they are used. E.g: https://github.com/iv-org/invidious/blob/2414e7db411f9de7ba40cad370d4b195830d4456/src/invidious/routes/account.cr#L101

Crystal Templates

These templates are mostly crystal code with some HTML in it. They don't use the ECR system because they have too much crystal conditionals and that would be hard to read otherwise.

One exception is for the comments templates. It's mixed with the API code and needs to be cleaned up really bad.

These templates are located here: https://github.com/iv-org/invidious/tree/2414e7db411f9de7ba40cad370d4b195830d4456/src/invidious/frontend

Components

One of the most important components is item.ecr. It's used in all grid lists (channels, playlist, search results, etc... It's described here: https://github.com/iv-org/invidious/blob/2414e7db411f9de7ba40cad370d4b195830d4456/src/invidious/views/components/item.ecr

Other components are miscellaneous snippets of HTML + Crystal code that are reused across different views. They're located in the same folder as item.ecr

mk-pmb commented 11 months ago

I finally made it work and see my own instance in my browser. 🎉

(Edit: Solved how to re-build.)

Also, how can I add attributes to the html and body tags? They seem to not be in the template.

iv-org / invidious