JeyRunner / tuda-typst-templates

Typst Template for the Corporate Design of TU Darmstadt
MIT License
17 stars 3 forks source link

Add tudaexercise template #8

Open FussballAndy opened 1 week ago

FussballAndy commented 1 week ago

tudaexercise typst template

Missing features

Optionals

FussballAndy commented 1 week ago

Ignore the commit history had some problems with git, we should just squash merge this PR

JeyRunner commented 1 week ago

Nice, it looks good. I added some review comments.

FussballAndy commented 1 week ago

Most review points should be fixed now. I also added a few more utilities for coloring and working with dicts/default values. One thing I am unsure about is whether we should give the user more customization options for coloring, so letting the user also be able to overwrite the background/text color from within the template. I am also thinking about adding a more general design state in which all design options like colors get stored. This will mostly just be used for the sections but will also give the user a way to access the colors used by the template.

JeyRunner commented 6 days ago

Looks pretty nice :+1: Regarding the customization options, you can add custom background/text colouring if you want, but I am not sure if it is really required. I would generally go for: simpler = better. But if it is simple to implement its fine I guess. In general, I would avoid using state if it is not needed. But your use case of giving the user access to computed colours makes sense (e.g. to make figures that match the accent colour). I would propose to make this design state just one way so that the user can't accidentally mess up the design by overwriting the state (and this way we effectively don't have state as long as the user does not read it). So essentially passing all computed colours to the sub-functions by value, but also writing the colours to the separate design state (but never read from it). What do you think?

Btw, you can also add your name to the typst.toml, so that you will be displayed as an author on the Universe website too.

FussballAndy commented 6 days ago

This should be mostly it. Though I am unsure if we should use the same README.md for both templates. Especially hence the images in these READMEs will also break in the typst packages repository. But also because (at least the current README) is rather specific to the thesis template.

JeyRunner commented 5 days ago

Yes looks pretty good. Regarding the readme, I think with a few adaptations we could make the readme more general. e.g. in 'Usage' change to:

Usage

Create a new typst project based on this template locally.

# for the thesis template
typst init @preview/athena-tu-darmstadt-thesis
cd athena-tu-darmstadt-thesis

# or for the exercise template
typst init @preview/athena-tu-darmstadt-exercise
cd athena-tu-darmstadt-exercise



There is already some functionality in the publish script to fix links which could be extended to all file links (just add the full path to this repo to all links).

But we could also have separate readmes per template. But I think most info is the same for all templates (disclaimer, font, logo setup).

FussballAndy commented 5 days ago

Yeah that sounds good. I did some changes to the README already. The script seems to only replace Markdown links and the images are embedded via img tags so I assume that is why that does not work right now. But that could just be done by hand for publishing. I will probably also remove some sections like TODOs and slightly change the main.typ setup but yeah for the rest the current README should be sufficient.

So from my end everything is done. Unless you have any more wishes or complaints I'd be open to this getting merged.

FussballAndy commented 2 days ago

Or actually I'd probably leave this open and submit it to universe first as there may be some suggested changes before it can get published. I can then add these changes to this PR without needing to create a new one. Or what do you think?

JeyRunner commented 1 day ago

Whatever you prefer. But I can also merge it now and then you could open a new pull request for the following changes.

JeyRunner commented 1 day ago

I added fixing of image links to the publish script (just pull the changes from the main branch). It should work now automatically.

FussballAndy commented 20 hours ago

Yeah then I'll probably submit it to universe first, doesn't really seem necessary to create a new PR for changes related to the first release.

FussballAndy commented 19 hours ago

I created a PR at https://github.com/typst/packages/pull/1314. Please also have a look at it.