Note that this is not Unplanned-Sprint-Work as it's a subtask of T165018: Page previews can consume new summary-HTML endpoint.

This feels like a styling issue caused by the addition of the p wrapper element to the container element, i.e. it's altering the size of the container element. I suspect that if we reset the styles of the wrapper element, then both issues will disappear.

@ovasileva I should remind everyone that the purpose of T165018 was to make it possible to show HTML - not to fix all the problems. That's why it was estimated at 3 points.

It worries me about the scope creep here. Given the need for design, should this really be in the current sprint? Personally I would like us to slow down, identify the problems in preparation for next sprint. There is a risk associated with adding unplanned work that has not been discussed by all the team.

@Jdlrobson - we discussed this during standup yesterday and today - looks like there might be a fix for this issue based on looking at line height that doesn't need any changes to design. In terms of planned/unplanned - we mentioned a few times during prioritization and planning that we will be including some of the bugs caused by T165018: Page previews can consume new summary-HTML endpoint into this sprint. The remainder of the bugs though aren't ready for work yet and are in needs analysis - most likely for next sprint.

In T168332#3365171, @ovasileva wrote:

@Jdlrobson - we discussed this during standup yesterday and today - looks like there might be a fix for this issue based on looking at line height that doesn't need any changes to design. In terms of planned/unplanned - we mentioned a few times during prioritization and planning that we will be including some of the bugs caused by T165018: Page previews can consume new summary-HTML endpoint into this sprint. The remainder of the bugs though aren't ready for work yet and are in needs analysis - most likely for next sprint.

Sure, but this is not a subtask of T16501 IMO and should be tagged unplanned sprint work. We did not plan to do this. This is a new bug that has been noticed as a side effect of enabling the summary endpoint and by pulling it in we are adding risk so should reflect this.

@Nirzar, as @phuedx says in T168332#3364440, the problem is arising from us setting a predefined height, but p elements having margins. Should we remove margins? If so, the problem will go away, but the paragraphs won't be visually separate, they'll read like one paragraph. I suppose this is fine as the extract is only a small bit of text.

Change 360415 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/extensions/Popups@master] Remove paragraph margins from HTML extracts

https://gerrit.wikimedia.org/r/360415

In T168332#3365224, @bmansurov wrote:

@Nirzar, as @phuedx says in T168332#3364440, the problem is arising from us setting a predefined height, but p elements having margins. Should we remove margins? If so, the problem will go away, but the paragraphs won't be visually separate, they'll read like one paragraph. I suppose this is fine as the extract is only a small bit of text.

@bmansurov - this seems like it would be close to the behavior we had before, or would there be other visual differences? If no, this is okay and we can discuss on restricting the extract to the first paragraph only later.

@ovasileva, the only difference will be the paragraphs won't be visually separated. I'm not sure of the old behavior, but I think this is an acceptable solution to the problem.

@bmansurov - the old behavior was the screenshot attached. Basically it would look like the image above? If so, yes, that's acceptable.

Oh, my bad. Yes, it will look like the old image.

In T168332#3365203, @Jdlrobson wrote:

Sure, but this is not a subtask of T16501 IMO…

You're absolutely right. It's definitely not a subtask of T16501: A way to hide the sidebar providing a "fullscreen" view ;)

and should be tagged unplanned sprint work. We did not plan to do this. This is a new bug that has been noticed as a side effect of enabling the summary endpoint and by pulling it in we are adding risk so should reflect this.

I think that this is a balanced approach to – loosely speaking – having an alpha version of HTML-driven previews up and running. We've noticed a minor regression – prior to QA even! – in the layout of previews caused by inserting HTML into 'em and we can fix it relatively quickly. This is by no means "all of the problems".

FTR @ovasileva, @pmiazga, and I all agreed that this is the only bug or task that should be brought into the sprint as a subtask of this task.

Change 360415 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Remove paragraph margins from HTML extracts

https://gerrit.wikimedia.org/r/360415

Here's what I see in the local environment when fetching previews from the RESTBase instance on the Beta Cluster 👍

I punted on trying to fix the class of layout bugs that occur when we talk about previews with multiple paragraphs. IMO an extract with multiple paragraphs seems… odd to me. I think we should discuss this as a team – including trying to find edge cases for whatever we land on – before taking this further.

@bmansurov, @phuedx - found an edge case:

1. Go to: https://en.wikipedia.beta.wmflabs.org/wiki/Category:Living_people
2. Hover over: Bill and Ruth Lucas

We should be stripping the disambiguation

@phuedx - related, although not a part of this task. What is the difference between the extract above (for Bill and Ruth Lucas) and the following:

1. Go to https://en.wikipedia.beta.wmflabs.org/wiki/Category:Living_people
2. Hover over Cryme Time

Here, we're seeing just the disambiguation, in the former, we're seeing the disambiguation and the first sentence.

In T168332#3370093, @ovasileva wrote:

@phuedx - related, although not a part of this task. What is the difference between the extract above (for Bill and Ruth Lucas) and the following:

The former (Bill and Ruth Lucas) has that text marked up as a list entry whereas the latter doesn't.

@ovasileva: In both cases, those sentences aren't marked up any meaningful way. Indeed, they aren't even marked up as hatnotes, which they are!

I think we're aware that including lists in page summaries is going to require a lot of additional work. I'm going to say that the first case is way, way out of scope here.

yup :(

I guess this relates to how we define "unplanned sprint work". To me this is unplanned and this might not seem important but I think it helps highlight "risky" tasks in a sprint. We did not plan to do this. This is a new bug that has been noticed as a side effect of enabling the summary endpoint and by pulling it in we are adding risk so should reflect this. I'm saying we should at least acknowledge it's unplanned. Anyway, continue as you are, maybe something for retro.

In T168332#3370091, @ovasileva wrote:

@bmansurov, @phuedx - found an edge case:

1. Go to: https://en.wikipedia.beta.wmflabs.org/wiki/Category:Living_people
2. Hover over: Bill and Ruth Lucas

We should be stripping the disambiguation

Yep definitely an editoral issue. To demonstrate how an editor would fix such an issue, I fixed it on wiki the 2 ways that are possible: https://en.wikipedia.org/w/index.php?title=Bill_and_Ruth_Lucas&type=revision&diff=786966027&oldid=747456605 AND/OR https://en.wikipedia.org/w/index.php?title=Bill_and_Ruth_Lucas&type=revision&diff=786966284&oldid=786966027

In T168332#3370093, @ovasileva wrote:

@phuedx - related, although not a part of this task. What is the difference between the extract above (for Bill and Ruth Lucas) and the following:

1. Go to https://en.wikipedia.beta.wmflabs.org/wiki/Category:Living_people
2. Hover over Cryme Time

Here, we're seeing just the disambiguation, in the former, we're seeing the disambiguation and the first sentence.

This has already been fixed in production - a reminder on why it's not always helpful to test on the beta cluster which uses outdated articles.

@Jdlrobson, @phuedx - sorry, I don't think I made my original question clear enough. I was wondering why the extract for Cryme Time is shorter/contains only one paragraph, while the extract for Bill and Ruth Lucas contains more. Infobox position maybe? Something else?

@ovasileva because of T168329

In T168332#3371352, @ovasileva wrote:

@Jdlrobson, @phuedx - sorry, I don't think I made my original question clear enough. I was wondering why the extract for Cryme Time is shorter/contains only one paragraph, while the extract for Bill and Ruth Lucas contains more. Infobox position maybe? Something else?

My previous explanation – that there was a section boundary marker present after the non-hatnote hatnote – was wrong. This is a symptom of T168329: exsentences does not work correctly when HTML output used.