search

clld / phoible

PHOIBLE Online
http://phoible.org
Apache License 2.0
42 stars 13 forks source link

Add FAQ & Conventions Page, and site-wide font change #30

Closed ltxom closed 3 years ago

ltxom commented 3 years ago

@xrotwang @drammock There are several things I made changes to this pull request.

  1. Add two new views of FAQ and Conventions pages in the phoible/__init__.py and phoible/view.py. Their Mako templates are phoible/templates/faq.mako and phoible/templates/conventions.mako. The Mako templates render views from static resources phoible/static/conventions.html and phoible/static/faq_with_indexes.html.
  2. In phoible/static/project.css file, I append CSS rules for (a) code highlighting styles, (b) site-wide style changes, and (c) Conventions page styles.
  3. In phoible/templates/phoible.mako file, I append a script that can (a) add Charis SIL CSS reference site-widely, (b) append menu items of Conventions and FAQ.
  4. Add two rules in the .gitignore file, which will ignore the .DS_STORE files in macOS and *.iml IntelliJ IDE confirgurations.

Please let me know if there is anything to fix. Thank you so much!

ltxom commented 3 years ago

@ltxom, I was hoping to get 2 separate PRs for this (see my last email): one that adds the two new static pages, and one for the changes that affect the whole site (like the CSS for the fonts). I requested that because I thought it would be easier for @xrotwang and I to review those contributions separately. We've now both reviewed, so no need to split into separate PRs at this point, but FYI in general it is often preferred to keep the changes in a PR semantically related (and avoid things like whitespace changes / touching unrelated files).

Sorry for the inconvenience! This is my first time making a PR. I misunderstood and thought should make 2 PR in two separate repositories. I will pay attention and do it correctly next time.

ltxom commented 3 years ago

@xrotwang @drammock Dear Robert and Dr. McCloy,

I submit a new commit that fixes the problems we discussed. Could you please check it for me? Let me know if there is any problem. Thank you!

drammock commented 3 years ago

LGTM. At some point over email we discussed changes to the homepage text, but let's do that in a separate PR.

ltxom commented 3 years ago

@xrotwang Hi Robert, Sorry! I misclicked the request review button. I was wondering how to merge the code into this repo once both of you approved.

xrotwang commented 3 years ago

I'll merge. You don't have sufficient permissions.

LTXOM @.***> schrieb am Do., 29. Apr. 2021, 19:55:

@xrotwang https://github.com/xrotwang Hi Robert, Sorry! I misclicked the request review button. I was wondering how to merge the code into this repo once both of you approved.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/clld/phoible/pull/30#issuecomment-829469011, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGUOKHVQ5EZQCXWFBX55F3TLGMQXANCNFSM43PJZNAQ .