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.

Sign up for GitHub

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

Jump to bottom

Update author footer styling #18

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

Update author footer styling #18

wants to merge 3 commits into from

Conversation

Cyan101
Copy link
Contributor

@Cyan101 Cyan101 commented Aug 3, 2017

Fixes Issue #17

@Cyan101 Cyan101 mentioned this pull request Aug 3, 2017
Open
@z64
Copy link
Contributor

z64 commented Aug 3, 2017

This style was intentional - I would like both names to be shown so that the reader will associate the author with both of them (on more platforms than just this blog), and I figured the CSS of the regular text vs. the link to their profile was enough to distinguish the two.

Additionally, the articles pages uses post.author, which will be their GitHub handle, so users looking for more posts from the same author may not know where to look.

If you want to distinguish the GitHub name more, other formats would be acceptable:

  • Zac Nowicki (z64)
  • Zac Nowicki [GitHub Logo] z64
  • z64 | Zac Nowicki

etc.

Either way, I would like both to be displayed.

@z64 z64 self-requested a review August 3, 2017 11:43
@Cyan101
Copy link
Contributor Author

Cyan101 commented Aug 3, 2017 •
edited
Loading

Having a | or using brackets or something would be nice i think, it looks weird saying:

Aria Edmonds aria

z64
z64 suggested changes Aug 3, 2017
View reviewed changes
Copy link
Contributor

@z64 z64 left a comment

Choose a reason for hiding this comment

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

Just marking this as changes requested per our convo above.

@z64 z64 changed the title Fixes #17 Update author footer styling Aug 3, 2017
@Cyan101
Copy link
Contributor Author

Cyan101 commented Aug 4, 2017

Don't merge yet please, on windows and so i can't double check how it looks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@z64 z64 z64 requested changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Cyan101 @z64