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

Press releases on standalone pages. #931

Merged
merged 3 commits into from
Nov 25, 2020

Conversation

jholdstock
Copy link
Member

@jholdstock jholdstock commented Nov 20, 2020

  • Individual press releases can now be linked to directly
  • /press does not have an ever-increasing list of hidden content
  • JavaScript is no longer required to view press release content

Also updated some of the existing .md files:

  • Rename some of the files so their URLs are more descriptive
  • Reformat to use consistent styling across each page

Demo site:

Closes #929

- Individual press releases can now be linked to directly
- /press does not have an ever-increasing list of hidden content
- JavaScript is no longer required to view press release content
@xaur
Copy link
Contributor

xaur commented Nov 21, 2020

Nice, thanks for picking this up!

Suggestions:

  • set page title to the title of the press release

    • it is currently "Decred - Press" for all releases
  • show published date on the page somewhere around the title

    • it is currently in the URL and in the hand-written text ("[Chicago, IL June 5, 2018]"), but explicit published date is always a good idea
  • if author metadata is available I would show it too to make the pages feel more real (writteb by real individuals)

@xaur
Copy link
Contributor

xaur commented Nov 21, 2020

Now that I look at Coverage and Press again I cannot ignore that top header and the info section below it eat ~550 px vertically. The actual Coverage and Press items (main elements of this page) start below the half of the vertical space on a big monitor, and are not visible at all without scrolling on a small laptop screen. I realize there are modern design trends and all that, but suppressing content to fit auxiliary elements sounds wrong. If it is out of scope for this PR I can move it into a new issue.

@jholdstock
Copy link
Member Author

Thanks for the review

  • Updated page title
  • Added published date on its own row
  • No author metadata is available so not displayed
  • Completely agree on the vertical spacing, removed some of the excess

Demo site updated:

https://dcrweb.jamieholdstock.com/press/
https://dcrweb.jamieholdstock.com/press/2020-10-21_dex_launch/

- The 404 page had a missing pageTitle, which only came to light when I made other changes in this PR.
- Other pages with missing pageTitles were highlighted, however these were pages we dont need (tags/categories) and old pages which were not cleaned from old output.
@xaur
Copy link
Contributor

xaur commented Nov 25, 2020

Looks great! A whole extra coverage/release item now fits vertically. I would suggest to also do something with the 345px-high header but that can be done in other PRs. This one lgtm.

P.S. Demo site is extremely helpful to quickly review changes.

@dajohi dajohi merged commit e52bf43 into decred:master Nov 25, 2020
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.

Press releases cannot be accessed with javascript off
3 participants