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

Author link visibility for blog and fix for avatar hover twitch #4354

Closed
wants to merge 10 commits into from
Closed

Author link visibility for blog and fix for avatar hover twitch #4354

wants to merge 10 commits into from

Conversation

rdela
Copy link
Contributor

@rdela rdela commented Mar 4, 2018

As discussed in #3199 I am splitting my PRs to the blog so the other one does not get unwieldy.

This one has:

  • avatars all linking to contrib. pages

  • add color to contributor page links

#3199 (comment)
#3199 (comment)

In addition as a special bonus this includes a fix for the twitchy avatars when hovering blog post previews that was driving me crazy. I have a video screen recording I will upload to my blog and add a link in a comment, the fix is:

marginTop: rhythm(17 / 8),
marginBottom: rhythm(17 / 8),

added to avatar wrapping div, and equal margins on Img:

marginRight: rhythm(1 / 32),
marginLeft: rhythm(1 / 32),

Added some developer beware comments around the danger zones.

also I hit the jackpot while building my test:

success createPages — 0.777 s

🎰

test output

Summary of all failing tests
 FAIL  packages/gatsby-source-wordpress/src/__tests__/normalize.js
  ● Test suite failed to run

    Cannot find module 'gatsby-source-filesystem' from 'normalize.js'

      2 | const deepMapKeys = require(`deep-map-keys`)
      3 | const _ = require(`lodash`)
    > 4 | const { createRemoteFileNode } = require(`gatsby-source-filesystem`)
      5 | 
      6 | const colorized = require(`./output-color`)
      7 | const conflictFieldPrefix = `wordpress_`
      
      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:194:17)
      at Object.<anonymous> (packages/gatsby-source-wordpress/src/normalize.js:4:34)


Test Suites: 1 failed, 72 passed, 73 total
Tests:       3 skipped, 512 passed, 515 total
Snapshots:   185 passed, 185 total
Time:        8.147s
Ran all test suites.

* upstream/master: (56 commits)
  check markdown images, ensure parent is File before referencing dir (#3831)
  Add "Dona Rita" website to Showcase (incl. source) (#3838)
  [www] Fix typo in styleshout link (#3839)
  Add name of file above code snippet (#3835)
  [www] add search input to home page (#3662)
  Added Hampton starter (#3826)
  Update README.md
  Update README.md (#3821)
  add warning about query that is not getting executed if it's exported from file that is page/template or layout (#3786)
  Log error in createPath (#3814)
  Added logos (#3781)
  Update README.md (#3828)
  Publish
  Call done() from dev-404-page during production builds so build doesn't stall (#3827)
  Fix a typo in the gatsby-plugin-less README (#3815)
  Publish
  format
  [gatsby-plugin-less] Extend less-plugin with support for `modifyVars` (#3801)
  Add iContract blog to readme (#3805)
  Publish
  ...
* upstream/master:
  docs: new intro to docs, add Part Zero, update names (#4153)
@gatsbybot
Copy link
Collaborator

gatsbybot commented Mar 4, 2018

Deploy preview for gatsbygram ready!

Built with commit 476f62f

https://deploy-preview-4354--gatsbygram.netlify.com

@rdela
Copy link
Contributor Author

rdela commented Mar 4, 2018

Edit 20180317: rm’d file using git filter-branch
https://help.github.com/articles/removing-sensitive-data-from-a-repository/

@rdela
Copy link
Contributor Author

rdela commented Mar 4, 2018

Also of note: this bumps title to the top which is so will match multi-author where title under author info is super weird. Never bothered me on single author until I noticed it, and Medium is really the only example I can find of author info on top style, but now I don’t like it on either personally.

* upstream/master:
  Publish
  Actually handle deletes fixes #4351 (#4353)
  docs: Clarify queries and Components (#4352)
@KyleAMathews
Copy link
Contributor

Seems to be cutting off the avatar now on hover https://deploy-preview-4354--gatsbyjs.netlify.com/blog/

Also, the spacing on blog previews changed quite a bit. Could you limit the css changes to the minimum necessary to fix the avatar twitch :-)

@rdela
Copy link
Contributor Author

rdela commented Mar 5, 2018

Hmmm the preview did fix the twitch for me and is not cutting avatars off for me either. What browser/OS are you seeing unwanted crop in? Screenshot for me maybe?

There are substantial spacing changes that are part of twitch remedy that maybe point to the fact I am solving a symptom and not root cause of twitch.

If I just back out spacing changes so we can get more noticeable contributor links in, and we live with the twitch for now, do you still want to keep the post title position change in this one or save that for multi-author PR?

Also depending on above want me to open a new, minimal PR on a clean branch without all the cruft in this one (some of which, namely GraphCMS post stuff, will end up in the tree later anyway). Or just add some commits because you are fine with it or will handle?

@rdela
Copy link
Contributor Author

rdela commented Mar 5, 2018

The twitching seems like a Chrome bug, Firefox and Safari do not exhibit the issue on my machine.

@KyleAMathews
Copy link
Contributor

Just in general, don't make design changes unless you clear that with me or @fk

Yeah, happy to have a PR fixing the twitch but I'd assume it'd be a fairly minor change.

Small PRs are best. If you want to work on finding the root cause of the twitch and solving that here that'd be great. If not, close this and start another minimal PR for adding multi-author.

@rdela rdela closed this Mar 6, 2018
@rdela rdela deleted the author-link-blog branch March 6, 2018 09:58
m-allanson pushed a commit that referenced this pull request Jun 27, 2018
…ws (#5813)

* Increase author link visibility and link avatars in blog posts + previews

Increase author link visibility and link avatar
- `www/src/components/blog-post-preview-item.js`
- `www/src/templates/template-blog-post.js`

Ref #3199 #4354 #5396

Confirmed addition of
`transform: translateZ(0)`
in  blog-post-preview-item.js prevents image twitch in Chrome
when hovering the card, way to go @fk

* lose font-size increase and bold

* lose font-size increase and bold for post too

* move keys that require !important to the && selector

...which glamor offers to increase specifity
(ref. https://github.com/threepointone/glamor/blob/master/docs/selectors.md)
via @fk review
`src/components/blog-post-preview-item.js`

* remove !important from blog preview and post layout
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.

3 participants