WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 3 months ago

#28967 accepted defect (bug)

Twenty Fourteen: Click on Continue Reading, and first line Covered

Reported by: Quantumstate Owned by: ianbelanger
Milestone: 5.3 Priority: normal
Severity: normal Version: 3.9
Component: Bundled Theme Keywords: has-patch needs-testing has-screenshots
Focuses: 2014 Cc:

Description

Theme 2014 Many problems, but insert <!--more-->. Now on the home page it shows as "Continue reading →". Click on Continue reading, and it takes you to the rest of the content, but the first line is covered by the blog name banner!

For example, at http://quantum-sci.com/cacook/ go down to "Our Microbiome" and read the excerpt. Naturally you click Read more, but the punchline is covered up. ("Guess the cure?")

Attachments (5)

28967.diff (467 bytes) - added by slobodanmanic 5 years ago.
28967.1.diff (551 bytes) - added by ianbelanger 5 months ago.
Refreshes patch and removes .masthead-fixed class in order to work on all browsers
28967.2.diff (707 bytes) - added by ianbelanger 4 months ago.
Patch refresh to fix some issues and support new editor more tag options
28967-gb-no-excerpt-full-page.PNG (6.3 KB) - added by ianbelanger 4 months ago.
28967-gb-no-excerpt-full-page-2.PNG (3.5 KB) - added by ianbelanger 4 months ago.
Without top padding on .entry-content

Download all attachments as: .zip

Change History (37)

#1 @SergeyBiryukov
5 years ago

  • Component changed from Themes to Bundled Theme
  • Summary changed from Click on Continue Reading, and first line Covered to Twenty Fourteen: Click on Continue Reading, and first line Covered

#2 @lancewillett
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Hello Quantumstate! Welcome to WordPress core Trac.

Thanks for the report.

#3 @Gwendydd
5 years ago

I have looked into this, and I can reproduce it.

The anchor is a span with an ID that is the same as the post ID (in this case, #more-75). The solution is to add something along these lines to that span:

	.admin-bar.masthead-fixed #more-75 {
		display: block; 
		position: relative; 
		top: -52px; 
		visibility: hidden;
	}

The complication is that you need to add that CSS only to the ID that is in the current URL, so you'll probably need JavaScript to do this. I don't have the JS chops to do that off the top of my head, but I will continue to investigate.

#4 follow-up: @Quantumstate
5 years ago

Thanks, but I don't know how to impliment that. It's a real annoyance as often key text is obscured and missed.

#5 in reply to: ↑ 4 @Gwendydd
5 years ago

Replying to Quantumstate:

Thanks, but I don't know how to impliment that. It's a real annoyance as often key text is obscured and missed.

I didn't expect you to know how to implement that. I was just leaving some notes about what I had discovered so that someone else could jump in and fix it. You're right that this is a problem, and needs to be fixed! Someone will take care of it.

@slobodanmanic
5 years ago

#6 @slobodanmanic
5 years ago

It was simple using CSS only:

.masthead-fixed .entry-content span[id*="more-"] {
	display: block;
	padding-top: 40px;
	margin-top: -64px;
}

.admin-bar.masthead-fixed .entry-content span[id*="more-"] {
	padding-top: 72px;
	margin-top: -96px;
}

Not sure if I should add any comments to these selectors, but rest of Twenty Fourteen's CSS file doesn't have them.

24px difference is to offset bottom margin for paragraph that wraps #more-xxx <span> element. According to caniuse.com selectors should work with IE8+ and all other browsers, but I haven't tested with IE8.

#7 follow-up: @Quantumstate
5 years ago

"Simple", the man says...

I've added these changes to my 2014 style.css, but I can't see as it's changed any. Still at least one line covered up by the header bar.

I don't understand the change so am reluctant to tweak it.

#8 in reply to: ↑ 7 @Gwendydd
5 years ago

Have you checked to see if you are viewing a cached version of the stylesheet? Try pressing control+shift+r to force your browser to download the latest stylesheet.

#9 @Quantumstate
5 years ago

Well, I'm seeing the fix in IE, but not in Firefox or Konqueror. Can someone please check my weblog and see what you see?

#10 @slobodanmanic
5 years ago

@Quantumstate by saying CSS fix was simple I didn't mean "this was so easy you should've done it yourself". Sorry if it sounded like that. I was working on a Javascript fix first, and compared to that CSS seems simple, at least to me.

#11 @Quantumstate
5 years ago

Understand slobodanmanic. I was just being ironic.

Thanks for all your work. I've seen alot of complaints about 2014 on IRC, but I think they all relate to styling issues which I've posted bugs on, and the gigantic article titles. Personally I've reduced the size of those titles, and made a number of other tweaks which I think make a very handsome blog. Just a few nits to pick.

#12 @Quantumstate
5 years ago

I give up. I tried.

#13 follow-up: @lancewillett
5 years ago

Does anyone know if this type of behavior has been discussed at the core level (for any theme)? I looked for related tickets but didn't find any right away.

#14 @helen
5 years ago

We handle it for the toolbar on load, although not in-page.

#15 in reply to: ↑ 13 @slobodanmanic
5 years ago

Replying to lancewillett:

Does anyone know if this type of behavior has been discussed at the core level (for any theme)? I looked for related tickets but didn't find any right away.

Doesn't it have to be handled on theme level if themes have sticky header?

#16 @chriscct7
4 years ago

  • Keywords needs-testing added

#17 follow-up: @karmatosed
3 years ago

Has anyone got a patch for this as would be great to revive and get a fix if it is a solution? We have a comment code snippet but no actual patch.

#18 in reply to: ↑ 17 @slobodanmanic
3 years ago

Replying to karmatosed:

Has anyone got a patch for this as would be great to revive and get a fix if it is a solution? We have a comment code snippet but no actual patch.

I attached the patch when I added the comment code snippet.

#21 @ianbelanger
6 months ago

  • Keywords bulk-reopened added

I first tested Twenty Fourteen to see if the problem still exists and it does. Then I attempted to test the current patch, however it no longer applies. So I added the suggested CSS to Twenty Fourteen's style.css and it does fix the issue in Firefox and Chrome, however this fix does not seem to work in any version of IE or Microsoft Edge. I'm currently working on a patch that will work in all browsers.

#22 @ianbelanger
5 months ago

  • Owner set to ianbelanger
  • Status changed from new to accepted

@ianbelanger
5 months ago

Refreshes patch and removes .masthead-fixed class in order to work on all browsers

#23 @ianbelanger
5 months ago

  • Keywords has-patch added; needs-patch removed

28967.1.diff is a solution that should work on all browsers. It is basically the same as @slobodanmanic's previous patch, but I had to remove masthead-fixed from the CSS. masthead-fixed is added dynamically by jquery only after scrolling, when a header image is being used. So in some browsers it was not added as a body class soon enough to be targeted.

This solution has been tested in Chrome, Firefox, IE11 and Edge on a Windows machine. It would be great to get someone to test this on other OS's and browsers.

#24 @ianbelanger
5 months ago

  • Milestone set to Future Release

#25 @ianbelanger
5 months ago

  • Milestone changed from Future Release to 5.2

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


5 months ago

#27 @desrosj
5 months ago

  • Keywords bulk-reopened removed

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#29 @andraganescu
4 months ago

  • Keywords commit added; needs-testing removed

I have tested this and it does solve the problem.

This ticket was mentioned in Slack in #core by ianbelanger. View the logs.


4 months ago

#31 @ianbelanger
4 months ago

  • Keywords needs-refresh added; commit removed
  • Milestone changed from 5.2 to 5.3

Punting to 5.3 milestone after some issues were found by @desrosj upon review

@ianbelanger
4 months ago

Patch refresh to fix some issues and support new editor more tag options

#32 @ianbelanger
4 months ago

So I think that 28967.2.diff should finally fix this issue, but I need someone with a Mac to test on various browsers. There is one caveat however, there is no margin between the paragraphs wherever the Read More tag is placed.

I also had to add some top padding to .entry-content in order to account for the new feature that allows you to hide the excerpt on the full content page. Screenshots coming.

@ianbelanger
4 months ago

Without top padding on .entry-content

#33 @ianbelanger
4 months ago

  • Keywords needs-testing has-screenshots added; needs-refresh removed

#34 @lgrev01
3 months ago

I run the patch on my macbook, but it doesn't show the excerpt. Can't find the read more button.

Note: See TracTickets for help on using tickets.