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

copyedits on frontpage #10157

Merged
merged 6 commits into from
Jan 18, 2024
Merged

copyedits on frontpage #10157

merged 6 commits into from
Jan 18, 2024

Conversation

benjaminxscott
Copy link
Contributor

@benjaminxscott benjaminxscott commented Jan 17, 2024

I can rebase btw

Before clicking "Create"

  • Branch is derived from the latest master
  • [NA] Add the pins label if this change will break existing games
  • Code passes linter with docker compose exec rack rubocop -a
  • Tests pass cleanly with docker compose exec rack rake

Implementation Notes

  • Explanation of Change

Goals:

  1. reduce amount of FAQ text in the 'yellow notification box', under the assumption that it should be for dynamic "update news"
  2. surface common answers e.g. "which discord servers exist" in a static FAQ section

As preliminary justification for some of these changes:

To save a few vertical pixels, I removed the Welcome <username> text since the user can view their login name/status in the top right (a common web idiom). Also changed the unit tests that check rendering.

I refactored the current text section into a FAQ section, while disabling the logic that hid it after a user created any games. Theoretically it should still be useful for users to see that info.

  • Screenshots

I wanted to make sure the text stack on the left was "about the same" height as the chat box, so users can still see their game list without having to scroll

image

  • Any Assumptions / Hacks

I'll do my best to detach my ego from these changes and any proposed changes / rejections of this PR

@tobymao tobymao merged commit 1d75c91 into tobymao:master Jan 18, 2024
1 check passed
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.

2 participants