Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix spacing problem of paragraphs around lists #10305

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions cypress/e2e/timeline/timeline.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,90 @@ describe("Timeline", () => {
);
});

it("should render lists with paragraphs", () => {
cy.visit("/#/room/" + roomId);

// Wait until configuration is finished
cy.contains(
".mx_RoomView_body .mx_GenericEventListSummary .mx_GenericEventListSummary_summary",
"created and configured the room.",
).should("exist");

// Send unordered & ordered lists with paragraphs
cy.sendEvent(roomId, null, "m.room.message" as EventType, {
msgtype: "m.text" as MsgType,
body:
"Paragraph above lists\n\n" +
"- item 1\n- item 2\n- item 3\n\n" +
"Paragraph between lists\n\n" +
"1. item 1\n2. item 2\n3. item 3\n\n" +
"Paragraph below lists",
format: "org.matrix.custom.html",
formatted_body:
"<p>Paragraph above lists</p>\n" +
"<ul>\n<li>item 1</li>\n<li>item 2</li>\n<li>item 3</li>\n</ul>\n" +
"<p>Paragraph between lists</p>\n" +
"<ol>\n<li>item 1</li>\n<li>item 2</li>\n<li>item 3</li>\n</ol>\n" +
"<p>Paragraph below lists</p>\n",
});

// Confirm the paragraph below the lists was rendered
cy.get(".mx_EventTile[data-layout=group] .markdown-body p:last-of-type").should(
"have.text",
"Paragraph below lists",
);

// Cypress `within` callback function to check block margins
// Note this cannot be used for compact modern layout
const checkBlockMargin = () => {
cy.get("ul").should("have.css", "margin-block", "0px");
cy.get("ol").should("have.css", "margin-block", "0px");
cy.get("p:first-of-type").should("have.css", "margin-block", "0px 16px"); // Paragraph above lists
cy.get("p:nth-of-type(2)").should("have.css", "margin-block", "16px"); // Paragraph between lists
cy.get("p:last-of-type").should("have.css", "margin-block", "16px 0px"); // Paragraph below lists
};

// Check block margin values of paragraphs and lists on modern layout
cy.get(".mx_EventTile[data-layout=group] .markdown-body").within(() => {
checkBlockMargin();
});

// Exclude timestamp and read marker from snapshot
const percyCSS = ".mx_MessageTimestamp, .mx_RoomView_myReadMarker { visibility: hidden !important; }";
cy.get(".mx_MainSplit").percySnapshotElement("Lists with paragraphs on modern layout", { percyCSS });

// Check block margin values of paragraphs and lists on compact modern layout
cy.setSettingValue("useCompactLayout", null, SettingLevel.DEVICE, true);
cy.get(".mx_EventTile[data-layout=group] .markdown-body").within(() => {
cy.get("ul").should("have.css", "margin-block", "0px 4px");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If ul and ol margin is in fact not expected here, this needs to be replaced as well. https://github.com/matrix-org/matrix-react-sdk/pull/10305/files#r1127766737

cy.get("ol").should("have.css", "margin-block", "0px 4px");
cy.get("p:first-of-type").should("have.css", "margin-block", "0px 4px"); // Paragraph above lists
cy.get("p:nth-of-type(2)").should("have.css", "margin-block", "4px"); // Paragraph between lists
cy.get("p:last-of-type").should("have.css", "margin-block", "4px 0px"); // Paragraph below lists
});
cy.setSettingValue("useCompactLayout", null, SettingLevel.DEVICE, false);

cy.get(".mx_MainSplit").percySnapshotElement("Lists with paragraphs on compact modern layout", {
percyCSS,
});

// Check block margin values of paragraphs and lists on IRC layout
cy.setSettingValue("layout", null, SettingLevel.DEVICE, Layout.IRC);
cy.get(".mx_EventTile[data-layout=irc] .markdown-body").within(() => {
checkBlockMargin();
});

cy.get(".mx_MainSplit").percySnapshotElement("Lists with paragraphs on IRC layout", { percyCSS });

// Check block margin values of paragraphs and lists on bubble layout
cy.setSettingValue("layout", null, SettingLevel.DEVICE, Layout.Bubble);
cy.get(".mx_EventTile[data-layout=bubble] .markdown-body").within(() => {
checkBlockMargin();
});

cy.get(".mx_MainSplit").percySnapshotElement("Lists with paragraphs on bubble layout", { percyCSS });
});

it("should set inline start padding to a hidden event line", () => {
sendEvent(roomId);
cy.visit("/#/room/" + roomId);
Expand Down
10 changes: 10 additions & 0 deletions res/css/views/rooms/_EventTile.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,11 @@ $left-gutter: 64px;
margin-top: 0;
Copy link
Member

Choose a reason for hiding this comment

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

(I don't really know why we need a margin-top here. As far as I can tell, it's the default anyway)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Me nether.

margin-bottom: 0;
}

/* Paragraph below ul or ol */
:is(ul, ol) + p {
margin-block: $spacing-16;
}
}
}

Expand Down Expand Up @@ -1308,6 +1313,11 @@ $left-gutter: 64px;
table {
margin-bottom: $spacing-4; /* 1/4 of the non-compact margin-bottom */
}

/* Apply to paragraph below ul or ol */
:is(ul, ol) + p {
margin-block: $spacing-4;
}
}
}

Expand Down