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 byline to puzzle page #444

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

SirMrGuy
Copy link

Adds the title and author name to the puzzle-solving interface. Includes the patron icon and adapts about as well as could be expected with absurdly long titles. Seems to adapt properly to odd aspect ratios, both of the window and puzzle. I elected to not make the author name link out directly so that mobile users don't accidentally tap it or something.

Resolves #391 (I think this is how github works)

Adds the title and author name to the puzzle-solving interface. Includes the patron icon and adapts about as well as could be expected with absurdly long titles. Seems to adapt properly to odd aspect ratios, both of the window and puzzle. I elected to not make the author name link out directly so that mobile users don't accidentally tap it or something.
Space for title bar no longer appears in the puzzle construction interface, so the puzzle is once again flush with the topbar
@mdirolf
Copy link
Member

mdirolf commented Sep 14, 2023

I'm so sorry for taking so long to review this. I was busy the week you posted it and then I honestly just completely forgot about it. I'm going to look into whether github has a setting to send a reminder if a PR has gone unacknowledged to hopefully keep this from happening again.

Your code changes look good! One issue is that we probably shouldn't be showing this for embeds where we already have the title in the top bar?

I'm not sure I'm 100% in favor of the UI change in general. I wonder what it would look like if we just added the title to the top bar to the right of the Crosshare logo (when there is enough room). As mentioned above we already do that for embeds so it'd be a pretty minor change to add it for first-party puzzles. Your change has the advantage of also showing the constructor's name but I'm not sure it's worth the loss of real estate.

Anyway, just trying to reopen discussion on this - again my apologies for the delay!

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.

Puzzle Titles in Grid Display
2 participants