Skip to content

feat(framework): make icons RTL aware #1833

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

Merged
merged 5 commits into from
Jun 22, 2020
Merged

feat(framework): make icons RTL aware #1833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
df58a12
feat: make icons RTL aware
ilhan007 Jun 18, 2020
0a4888b
update test page
ilhan007 Jun 18, 2020
e8ab10f
address comments
ilhan007 Jun 19, 2020
ad149ad
add eslint-disable-line
ilhan007 Jun 19, 2020
6d86faf
fix the other trmplate quotes
ilhan007 Jun 19, 2020
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
4 changes: 2 additions & 2 deletions packages/base/src/SVGIconRegistry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ const calcKey = (name, collection) => {
return `${collection}:${name}`;
};

const registerIcon = (name, { pathData, accData, collection } = {}) => {
const registerIcon = (name, { pathData, ltr, accData, collection } = {}) => { // eslint-disable-line
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the eslint ignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am getting these (although I am adding one additional key):
17:29 error Expected a line break after this opening brace object-curly-newline
17:66 error Expected a line break before this closing brace object-curly-newline

const key = calcKey(name, collection);
registry.set(key, { pathData, accData });
registry.set(key, { pathData, ltr, accData });
};

const getIconDataSync = (name, collection = DEFAULT_COLLECTION) => {
Expand Down
9 changes: 8 additions & 1 deletion packages/base/src/asset-registries/Icons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@ const registerIconBundle = async (collectionName, bundleData) => {

const fillRegistry = bundleData => {
Object.keys(bundleData.data).forEach(iconName => {
registerIcon(iconName, { pathData: bundleData.data[iconName], accData: bundleData.accData[iconName], collection: bundleData.collection });
const iconData = bundleData.data[iconName];

registerIcon(iconName, {
pathData: iconData.path,
ltr: iconData.ltr,
accData: iconData.acc,
collection: bundleData.collection,
});
});
};

Expand Down
2,725 changes: 2,064 additions & 661 deletions packages/icons/src/icon-collections/SAP-icons.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/main/src/CheckBox.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
>
<div id="{{_id}}-CbBg" class="ui5-checkbox-inner">
{{#if checked}}
<ui5-icon name="accept" class="ui5-checkbox-icon" dir="ltr"></ui5-icon>
<ui5-icon name="accept" class="ui5-checkbox-icon"></ui5-icon>
{{/if}}

<input id="{{_id}}-CB" type='checkbox' ?checked="{{checked}}" ?readonly="{{readonly}}" ?disabled="{{disabled}}" data-sap-no-tab-ref/>
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/Icon.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<svg
class="ui5-icon-root"
tabindex="{{tabIndex}}"
dir="{{effectiveDir}}"
dir="{{_dir}}"
viewBox="0 0 512 512"
role="{{role}}"
focusable="false"
Expand Down
13 changes: 13 additions & 0 deletions packages/main/src/Icon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,18 @@ class Icon extends UI5Element {
}
}

get _dir() {
if (!this.effectiveDir) {
return;
}

if (this.ltr) {
return "ltr";
}

return this.effectiveDir;
}

get tabIndex() {
return this.interactive ? "0" : "-1";
}
Expand Down Expand Up @@ -249,6 +261,7 @@ class Icon extends UI5Element {

this.pathData = iconData.pathData;
this.accData = iconData.accData;
this.ltr = iconData.ltr;
}

get hasIconTooltip() {
Expand Down
83 changes: 80 additions & 3 deletions packages/main/test/pages/Icon.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
<script src="../../resources/bundle.esm.js" type="module"></script>
<script nomodule src="../../resources/bundle.es5.js"></script>

<style>
section {
border: dashed 2px red;
}
</style>
</head>

<body style="background-color: var(--sapBackgroundColor);">
Expand All @@ -43,7 +48,7 @@
<div id="content"></div>
<ui5-icon name="appointment-2"></ui5-icon>
<ui5-icon name="appointment-2"></ui5-icon>
<!-- <ui5-icon name="appointment-2"></ui5-icon>
<ui5-icon name="appointment-2"></ui5-icon>
<ui5-icon name="appointment-2"></ui5-icon> -->

<br />
Expand Down Expand Up @@ -115,7 +120,7 @@
iconNames.forEach(iconName => {
var icon = document.createElement("ui5-icon");
icon.name = iconName;
document.body.appendChild(icon);
allIcons.appendChild(icon);
});
})();
</script>
Expand All @@ -125,10 +130,82 @@
iconNames.forEach(function(iconName) {
var icon = document.createElement("ui5-icon");
icon.name = iconName;
document.body.appendChild(icon);
allIcons.appendChild(icon);
});
});
</script>


<section id="allIcons">
<h3>All icons</h3>
</section>

<section class="sectionLTR1" dir="ltr">
<h3>Icons in ltr</h3>
<ui5-icon name="accept"></ui5-icon>
<ui5-icon name="sales-document"></ui5-icon>
<ui5-icon name="sales-notification"></ui5-icon>
<ui5-icon name="search"></ui5-icon>
<ui5-icon name="simple-payment"></ui5-icon>
<ui5-icon name="sound-loud"></ui5-icon>
<ui5-icon name="sound-off"></ui5-icon>
<ui5-icon name="sound"></ui5-icon>
<ui5-icon name="sys-help"></ui5-icon>
<ui5-icon name="text-align-justified"></ui5-icon>
<ui5-icon name="text-align-center"></ui5-icon>
<ui5-icon name="text-align-left"></ui5-icon>
<ui5-icon name="text-align-right"></ui5-icon>
<ui5-icon name="text-formatting"></ui5-icon>
<ui5-icon name="line-chart-time-axis"></ui5-icon>
<br>
<ui5-checkbox checked></ui5-checkbox>
</section>

<section class="sectionRTLNotMirrored" dir="rtl">
<h3>Icons in RTL, but not mirrored</h3>
<ui5-icon name="accept"></ui5-icon>
<ui5-icon name="sales-document"></ui5-icon>
<ui5-icon name="sales-notification"></ui5-icon>
<ui5-icon name="search"></ui5-icon>
<ui5-icon name="simple-payment"></ui5-icon>
<ui5-icon name="sound-loud"></ui5-icon>
<ui5-icon name="sound-off"></ui5-icon>
<ui5-icon name="sound"></ui5-icon>
<ui5-icon name="sys-help"></ui5-icon>
<ui5-icon name="text-align-justified"></ui5-icon>
<ui5-icon name="text-align-center"></ui5-icon>
<ui5-icon name="text-align-left"></ui5-icon>
<ui5-icon name="text-align-right"></ui5-icon>
<ui5-icon name="text-formatting"></ui5-icon>
<ui5-icon name="line-chart-time-axis"></ui5-icon>
<br>
<ui5-checkbox checked></ui5-checkbox>
</section>

<section class="sectionLTR2" dir="ltr">
<h3>Icons in LTR</h3>
<ui5-icon name="slim-arrow-left"></ui5-icon>
<ui5-icon name="slim-arrow-right"></ui5-icon>
<ui5-icon name="media-play"></ui5-icon>
<ui5-icon name="media-reverse"></ui5-icon>
<ui5-icon name="nav-back"></ui5-icon>
<ui5-icon name="trend-down"></ui5-icon>
<ui5-icon name="trend-up"></ui5-icon>
<ui5-icon name="undo"></ui5-icon>
<ui5-icon name="step"></ui5-icon>
</section>

<section class="sectionRTLMirrored" dir="rtl">
<h3>Icons in RTL and mirrored</h3>
<ui5-icon name="slim-arrow-left"></ui5-icon>
<ui5-icon name="slim-arrow-right"></ui5-icon>
<ui5-icon name="media-play"></ui5-icon>
<ui5-icon name="media-reverse"></ui5-icon>
<ui5-icon name="nav-back"></ui5-icon>
<ui5-icon name="trend-down"></ui5-icon>
<ui5-icon name="trend-up"></ui5-icon>
<ui5-icon name="undo"></ui5-icon>
<ui5-icon name="step"></ui5-icon>
</section>
</body>
</html>
26 changes: 13 additions & 13 deletions packages/tools/lib/create-icons/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,39 @@ const destDir = `dist/icons/`;

mkdirp.sync(destDir);

const template = (name, pathData) => `import { registerIcon } from "@ui5/webcomponents-base/dist/SVGIconRegistry.js";
const template = (name, pathData, ltr) => `import { registerIcon } from "@ui5/webcomponents-base/dist/SVGIconRegistry.js";

const name = "${name}";
const pathData = "${pathData}";
const ltr = ${ltr};

registerIcon(name, { pathData });
registerIcon(name, { pathData, ltr});

export default { pathData };`;

const accTemplate = (name, pathData, accData) => `import { registerIcon } from "@ui5/webcomponents-base/dist/SVGIconRegistry.js";
const accTemplate = (name, pathData, ltr, accData) => `import { registerIcon } from "@ui5/webcomponents-base/dist/SVGIconRegistry.js";
import { ${accData.key} } from "../generated/i18n/i18n-defaults.js";

const name = "${name}";
const pathData = "${pathData}";
const ltr = ${ltr};
const accData = ${accData.key};

registerIcon(name, { pathData, accData });
registerIcon(name, { pathData, ltr, accData });

export default { pathData, accData };`;


const createIcons = (file) => {
const json = JSON.parse(fs.readFileSync(file));

for (let name in json.data) {
let content;
const pathData = json.data[name];
const accData = json.accData[name];

if (accData) {
content = accTemplate(name, pathData, accData);
} else {
content = template(name, pathData);
}
const iconData = json.data[name];
const pathData = iconData.path;
const ltr = !!iconData.ltr;
const acc = iconData.acc;

const content = acc ? accTemplate(name, pathData, ltr, acc) : template(name, pathData, ltr);

fs.writeFileSync(path.join(destDir, `${name}.js`), content);
}
Expand Down