neroden / timetable_kit

A Python toolkit for generating human-readable timetables from GTFS data; uses PANDAS and gtfs_kit
GNU Affero General Public License v3.0
39 stars 2 forks source link

Configurable styling (with an interest in VIA) #21

Open Borketh opened 6 months ago

Borketh commented 6 months ago

This is just a start, and doesn't include any content changes (24h time and different ARDP handling), only CSS. Crucially, this does not actually change anything except the colour of the VIA h1 to their yellow colour. It's supposed to make the timetables look like this testing timetable I tweaked manually: image However, it does not seem to be properly applying the via-special-css class into the table elements themselves. Obviously, this is an inherently inefficient way to try this in the first place. Ideally, the style would be configurable by switching between entire stylesheets and/or processes entirely. I'm kind of stuck as I have limited knowledge of Jinja. How would you suggest this be done? Relevant documentation would be appreciated.

For reference, this is the VIA style guide. Oddly enough, their own website uses a different shade of yellow...

Borketh commented 6 months ago

My intention here is that CSS snippets be dynamically loadable loadable instead of front loading everything and deciding what style gets used with umpteen different tags. This would also mean completely unnecessary CSS could be left out of a given timetable (for example, the Hudson Bay timetable will never need to know how to style the MBTA logo, but that CSS is still added right now).

neroden commented 6 months ago

Yeah, the current system for assembling CSS is a bit brute-force. There are several considerations. I've been trying to do the semantic separation properly in HTML vs CSS (so that the stylesheet can be overridden) and that took me a while to get even approximately right.

At this point you really should just be able to apply a cascading stylesheet which is prioritized over the built-in one (as a user stylesheet with your client browser). I haven't tested it -- I meant to. If it doesn't work that's a bug.

Part of the problem is that you seem to have some confusion regarding good CSS selector practice. CSS selectors are... weird and counterintuitive. I suggest reviewing their bizarre syntax and semantics. I think you want to add "via-special-css" to the tag for the table, or better yet the

tag for the entire page, and then catch descendants of it in your stylesheet, rather than adding the tag to every single element, which is what you're trying to do.

The elements are mostly tagged being functionally (type of cell, etc.), though there are some leftovers (row and column number tracking) copied from the old PANDAS code.

I was planning to provide an option for an external rather than embedded stylesheet but then the HTML isn't self-contained and aaaargh this has its own problems when I go to push the file through the PDF converter.

The icon & logo measurement stuff is incredibly tedious, since it's really not stuff which belongs in CSS but that's how things work with modern HTML/CSS (I tried several other approaches to get the measurements into the HTML and they all had fatal flaws of one sort or another). This is actually one of the major obstructions to a clean style vs. substance separation.

I could probably collect the relevant CSS fragments for connecting services icons the way I collect the relevant connecting services for producing the HTML list -- note that I also currently ship all the connecting services icon files, relevant or not, and the same collection-list approach is probably desirable. This could probably be done relatively elegantly in the connecting services module. Have a function in the module, to which you pass a list of connecting services back into the module and have it return the CSS. Have another function to which you pass as list of connecting services and have it return the list of icon files.

Don't put the if-then logic into the RPA logo file. I'm trying to separate the HTML which describes an image and how to present it from the logic which decides whether or not to include it. Makes it easier if they change the frickin' logo or if I start having other sponsor logos to choose from. (I would have a whole subdirectory of these, like the connecting services files... if there were more than one. But it's not worth overengineering.)

Anyway those are just some incomplete design notes. I wish I had the time and energy to work on this myself right now.

Borketh commented 6 months ago

I think you want to add "via-special-css" to the tag for the table, or better yet the tag for the entire page, and then catch descendants of it in your stylesheet, rather than adding the tag to every single element, which is what you're trying to do.

Oh right, that's a thing. Can you tell frontend isn't my forte?

Have a function in the module, to which you pass a list of connecting services back into the module and have it return the CSS. Have another function to which you pass as list of connecting services and have it return the list of icon files.

Makes sense. Will you do that or do I?

Don't put the if-then logic into the RPA logo file.

Good call, that was an earlier change I made before I fully understood the loading. Should the rpa_logo be more generic in future so that we can accommodate other logos (such as Transport Action Canada).

Anyway those are just some incomplete design notes. I wish I had the time and energy to work on this myself right now.

This is very helpful though, thank you for taking the time to explain this! I'll try to find the time to work on it after this round of school deliverables.

neroden commented 6 months ago

I think you want to add "via-special-css" to the tag for the table, or better yet the tag for the entire page, and then catch descendants of it in your stylesheet, rather than adding the tag to every single element, which is what you're trying to do.

Oh right, that's a thing. Can you tell frontend isn't my forte?

CSS is very... unusual... and I had to spend a lot of time relearning CSS to get any of this done. Not my forte either really!

Have a function in the module, to which you pass a list of connecting services back into the module and have it return the CSS. Have another function to which you pass as list of connecting services and have it return the list of icon files.

Makes sense. Will you do that or do I?

Depends who gets to it first, right? If you get to it before I do I'll probably tweak it, of course.

Good call, that was an earlier change I made before I fully understood the loading. Should the rpa_logo be more generic in future so that we can accommodate other logos (such as Transport Action Canada).

Yeah, it should probably be something like "sponsor" with a selection of sponsors. Good thought

Borketh commented 5 months ago

@neroden I'd like to hear your thoughts on how I would implement agency specific content inclusion, such as times being in 12/24hr format, the text "Train #" above tsns, and extra station information such as station titles.

neroden commented 5 months ago

Ahhh, yes. This stuff.

So here's an underlying issue: my division into agencies with agency-specific code was designed around reading different GTFS files, because each agency has its own quirks of GTFS format. Which is necessary.

What we're looking at here is a different distinction -- just as I can present VIA timetables with Amtrak style, we could present Amtrak timetables with VIA style. (Or timetables in another house style for New York by Rail... I was considering discussing that with the guy who publishes that.)

I think what we arguably need is a --style parameter to the main run of timetable.py. While some of the differences end up in the stylesheet, as you've pointed out, others don't, they end up in content.

I'm not entirely sure how to orthogonally implement the style options, but I think it needs to be a separate parameter passed to the main code, so that choices like the time format can be switched off of that in the main codebase.

But... maybe it shouldn't be a command-line parameter. After all, style is what the spec files are all about. Maybe it should be a parameter in the .toml file. I like that, actually.

Borketh commented 5 months ago

I think both would work. We could have the .toml file say what the default style should be but be able to override that with a --style parameter.

So how should this be implemented? I'm thinking something like parallel classes to Agency. AgencyStyle, AmtrakStyle, ViaStyle, etc. Should that be a subclass or member of Agency, or not related at all? My preference would be class member referencing the relevant style class. The agency_special_css and possibly other future methods of handling the agency-specific css could go there as well.

neroden commented 5 months ago

I'm pretty sure it should be totally separate from the agency classes.

We might have an RPA House Style or a New York By Rail House Style or a "Large Print Wall Poster" style or an "old-timey 1890s-imitation" style... you get the picture.

Since the current default style I'm producing isn't actually exactly the Amtrak style, even though it's based on it, I'd call it Nathanael's house style, "default style", or "standard style".

-- Nathanael C. Nerode @.***

On Mon, Jun 17, 2024, at 4:12 AM, Borketh wrote:

I think both would work. We could have the .toml file say what the default style should be but be able to override that with a --style parameter.

So how should this be implemented? I'm thinking something like parallel classes to Agency. AgencyStyle, AmtrakStyle, ViaStyle, etc. Should that be a subclass or member of Agency, or not related at all? My preference would be class member referencing the relevant style class. The agency_special_css and possibly other future methods of handling the agency-specific css could go there as well.

-- Reply to this email directly or view it on GitHub: https://github.com/neroden/timetable_kit/pull/21#issuecomment-2172590158 You are receiving this because you were mentioned.

Message ID: @.***>

Borketh commented 3 months ago

@neroden got a chance to work on this again. What are your thoughts on this initial API draft?

neroden commented 3 months ago

My initial thoughts are highly positive. I like it a lot, though I haven't had a chance to work on timetable_kit and I don't have a chance to do a full code review right now. (There's too much which needs to be done on Covid safety right now, which is life and death, and I have a bunch of personal stuff going on too.)

You noticed how long the day string code was? I had SO MANY tweak requests for that before all the Amtrak fans were satisfied.

-- Nathanael C. Nerode @.***

On Sat, Aug 10, 2024, at 1:44 AM, Borketh wrote:

@neroden got a chance to work on this again. What are your thoughts on this initial API draft?

-- Reply to this email directly or view it on GitHub: https://github.com/neroden/timetable_kit/pull/21#issuecomment-2279469892 You are receiving this because you were mentioned.

Message ID: @.***>

Borketh commented 3 months ago

You noticed how long the day string code was? I had SO MANY tweak requests for that before all the Amtrak fans were satisfied.

Yeah lol, it's a lot. Like I said in the comment I left, there's probably a slightly better way to do it but it's not worth the effort right now lol.

There's too much which needs to be done on Covid safety right now, which is life and death, and I have a bunch of personal stuff going on too.

Good luck! No rush to merge anything. I'm still working on integrating it into the rest of the codebase, and I don't want to accidentally remove functionality when it's ready.

Borketh commented 3 months ago

Here's the difference a simple flag now makes (--style via-tac) to the Canadian's timetable. The only issue is now the default style overrides the owner of the route (lmao) because that's tied to agency-special-css. That should probably be retied to the agency classes themselves rather than being inserted by CSS with that class.

image image

Borketh commented 3 months ago

One thing I'd like to mention: all amtrak timetables seem to be broken and I don't know how to diagnose the issue. No trains are present in the GTFS by the time it gets to look for the ones it needs for a given timetable. Maybe it's a code issue, maybe it's a reference date issue, maybe the feed itself is faulty - I have no idea. Just a heads up.

Borketh commented 3 months ago

This is going to be a big review, and I look forward to the epic roast of some of the stuff I did. I suggest going per-commit because taking it in as the sum of all commits looks like a nightmare. There's no rush to get this merged, as I can generate what I need for TAC from my fork. Good luck with the things you're dealing with right now!

neroden commented 2 months ago

You know, I love this. I still haven't had time for code review (there is so. much. other. stuff. going on in my life) but I'm going to try to make time soon because I want to integrate this. It looks really good.

-- Nathanael C. Nerode @.***

On Sat, Aug 24, 2024, at 5:59 AM, Borketh wrote:

Here's the difference a simple flag now makes (--style via-tac) to the Canadian's timetable. The only issue is now the default style overrides the owner of the route (lmao) because that's tied to agency-special-css. That should probably be retied to the agency classes themselves rather than being inserted by CSS with that class.

image image

-- Reply to this email directly or view it on GitHub: https://github.com/neroden/timetable_kit/pull/21#issuecomment-2308293057 You are receiving this because you were mentioned.

Message ID: @.***>

Borketh commented 2 months ago

That's great to hear! I don't have much time to work on this now that university is back in session. I would like to get the amtrak issues fixed before this is merged but I'm not sure anything I did in this PR is the problem. Needs further investigation...

neroden commented 1 week ago

I'm afraid I don't know when I'll be able to review any of this. I currently have to hire a lawyer to protect my partner's health from people trying to force her to get infected with deadly disease.

Keep your respirator mask (N95, P100) on.

-- Nathanael C. Nerode @.***

On Wed, Sep 25, 2024, at 10:08 AM, Borketh wrote:

That's great to hear! I don't have much time to work on this now that university is back in session. I would like to get the amtrak issues fixed before this is merged but I'm not sure anything I did in this PR is the problem. Needs further investigation...

-- Reply to this email directly or view it on GitHub: https://github.com/neroden/timetable_kit/pull/21#issuecomment-2374198483 You are receiving this because you were mentioned.

Message ID: @.***>