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

Add pdfs.hbs [WIP] #118

Merged
merged 5 commits into from
Oct 21, 2024
Merged

Add pdfs.hbs [WIP] #118

merged 5 commits into from
Oct 21, 2024

Conversation

cstby
Copy link
Contributor

@cstby cstby commented Oct 14, 2024

This is a first shot at issue #117.

A few notes:

  • I didn't include the bleed pdf because I didn't see a use case for the normal user. The only time one would need a bleed pdf would be for professional printing, correct? Including the bleed pdf is trivial, so if I'm mistaken in understanding the use case, I'm happy to add it.
  • I used the existing styles from stylesheet-index.css and avoided adding more code to the css file. This means that added a few manual breaks into the hbs template. I believe this is simpler and more maintainable than editing the css to account for the flexbox spacing.
  • I used good judgment on the copy, but I'm not attached to it if we want to change it.

Any and all feedback is welcome!

Copy link

github-actions bot commented Oct 14, 2024

surge: deployed to https://liputenpotest-118.surge.sh
surge: deployed to https://liputenpotest-118.surge.sh
surge: deployed to https://liputenpotest-118.surge.sh
surge: deployed to https://liputenpotest-118.surge.sh
surge: removed the deployment on https://liputenpotest-118.surge.sh

@alifeee
Copy link
Member

alifeee commented Oct 14, 2024

toki a! pona mute tawa sina tan pali sina a

this looks great :)

as the bot says, we can find a preview on https://liputenpotest-118.surge.sh/ (but because of filesize, the preview does not have PDFs)

when I made the issue, I was just imagining a pretty plaintext-list, but the images and layout makes it look nice :)

a few notes:

  • "Color A5 PDF" etc. is in English. It should be TP, e.g., "lipu kule", "lipu tawa ilo sitelen", "lipu pimeja/walo tawa ilo sitelen"
  • I don't know that specifying A5/A4 is that helpful (and in some cases like nanpa lili, I believe wrong). This could be left out
  • there is not necessarily the three types of PDF for each issue. For example
    • many issues have an extra PDF: the sitelen pona version !
    • some issues do not have print versions, e.g., nanpa kijetesantakalu / kati tiki tu lili
      thus, we should use data from lipu_ale.yaml and use conditionals, as we do on the lipu pages
  • the word "PDF" interspaced with toki pona words seems strange to me. perhaps lipu is fine. otherwise, the copy is nice :)

overall this is a wonderful change ! thanks for giving it an attempt :)

I hope you find eleventy to your pleasure.

with changes to the note of my notes, this is easily ready to merge and a good page (almost an easter egg of a page ;)

Copy link
Member

@alifeee alifeee left a comment

Choose a reason for hiding this comment

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

a final note: I prefer the header to stay lipu rather than lipu pdf - I hope this is understandable :)

@cstby
Copy link
Contributor Author

cstby commented Oct 15, 2024

Thanks again for the quick feedback!

  • I changed the copy as you suggested (removing "pdf" and using toki pona instead of Engilsh).
  • I also added conditional logic for each pdf type, including adding sitelen pona.

I noticed that the long copy "lipu walo tawa ilo sitelen" used the full width and spilled over onto a second line. My final commit adds some styling changes to make each lipu wider so that each pdf link only uses one line. I think this looks a little better, but it's totally up to you whether you'd prefer not to add any additional CSS (or to go a different direction with the design of this page).

@alifeee
Copy link
Member

alifeee commented Oct 16, 2024

the CSS is fine with me!

this looks ready to merge !!

only one thing I should have said before: I think we should put a link at the top of the page that says "download all" or similar (in toki pona), which links to https://github.com/lipu-tenpo/liputenpo.org/tree/main/pdfs

after that I will merge it :) thank you for doing this

@cstby
Copy link
Contributor Author

cstby commented Oct 21, 2024

Added in that link in a new line in the introduction section!

@alifeee
Copy link
Member

alifeee commented Oct 21, 2024

I have made a small change to move the URL to sona.yaml (in _data), so as to not have implementation-specific (GitHub) stuff in the HTML templates.

thanks a lot for this !! I will merge and it should be visible on the site soon :)

@alifeee alifeee merged commit ece5040 into lipu-tenpo:main Oct 21, 2024
1 check passed
@alifeee alifeee linked an issue Oct 21, 2024 that may be closed by this pull request
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.

make a page for /pdfs/
2 participants