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

MWPW-148439 - FIX - Text wraps under icon & No space between Icon and text in RTL Locale. #2343

Merged
merged 3 commits into from
Jun 3, 2024
Merged

MWPW-148439 - FIX - Text wraps under icon & No space between Icon and text in RTL Locale. #2343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d86264
fix: fixed text wrap;fixed icon space issue when RTL enabled
sivasadobe May 21, 2024
036b352
fix: removed old browser support
sivasadobe May 22, 2024
2e04788
Update libs/styles/styles.css
sivasadobe May 31, 2024
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
5 changes: 5 additions & 0 deletions libs/blocks/icon-block/icon-block.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@
margin-top: var(--spacing-s);
}

.icon-block .foreground .second-column .title-row {
display: flex;
align-items: center;
}
Comment on lines +180 to +183
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you're already using flex, couldn't a column-gap be used instead of the more complex margin logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is primarily used to fix the text under the icon.
Yes, column-gap will solve this problem on pages written by ICON BLOCK. However, if RTL is enabled, it will not fix icon spacing on other pages.
The goal is to solve the no space problem on all milo pages. As a result, we changed the margin in icon.js to margin-inline, allowing the margin to be added based on direction.


.icon-block.full-width.small .foreground .icon-area,
.icon-block.vertical.small .foreground .icon-area {
margin-bottom: var(--spacing-s);
Expand Down
10 changes: 9 additions & 1 deletion libs/blocks/icon-block/icon-block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,15 @@ function decorateContent(el) {
const textContent = el.querySelectorAll('.text-content > :not(.icon-area)');
const secondColumn = createTag('div', { class: 'second-column' });
textContent.forEach((content) => {
secondColumn.append(content);
let nodeToInsert = content;
const firstIcon = content.querySelector('.icon:first-child');
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be sure: you're targeting the icon that is the first sibling within its parent, right? If it had a text before it (or any other element), this would no longer select it and apply the logic. Is that the expectation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we need to select the first icon of the space and move the child of its parent tag, only after that we can do the styling. If there is no icon at first, we don't need to do anything.

if (firstIcon) {
const titleRowSpan = createTag('span', { class: 'title-row' });
titleRowSpan.append(firstIcon, content);
nodeToInsert = titleRowSpan;
}

secondColumn.append(nodeToInsert);
});
if (secondColumn.children.length === 1) el.classList.add('items-center');
el.querySelector('.foreground .text-content').append(secondColumn);
Expand Down
6 changes: 3 additions & 3 deletions libs/features/icons/icons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ export default async function loadIcons(icons, config) {
const parent = icon.parentElement;
if (parent.childNodes.length > 1) {
if (parent.lastChild === icon) {
icon.classList.add('margin-left');
icon.classList.add('margin-inline-start');
} else if (parent.firstChild === icon) {
icon.classList.add('margin-right');
icon.classList.add('margin-inline-end');
if (parent.parentElement.tagName === 'LI') parent.parentElement.classList.add('icon-list-item');
} else {
icon.classList.add('margin-left', 'margin-right');
icon.classList.add('margin-inline-start', 'margin-inline-end');
}
}
icon.insertAdjacentHTML('afterbegin', iconSVGs[iconName].outerHTML);
Expand Down
2 changes: 2 additions & 0 deletions libs/styles/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,8 @@ span.icon.margin-left { margin-left: 8px; }

span.icon.margin-inline-end { margin-inline-end: 8px; }

span.icon.margin-inline-start { margin-inline-start: 8px; }

.button-l .con-button span.icon.margin-left,
.con-button.button-l span.icon.margin-left { margin-left: 12px; }

Expand Down
Loading