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.

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

Add context to provide rtl support for react-icons #560

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
31c7a3f
Automate bumping react-icons beta version number
tomi-msft Nov 23, 2021
47fae8d
revert
tomi-msft Nov 23, 2021
8f81a86
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Nov 23, 2021
179a3e7
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Nov 30, 2021
55e3032
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Dec 6, 2021
1cfdd33
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Feb 8, 2022
66d5a05
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Mar 1, 2022
c84e246
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Mar 10, 2022
80bef55
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Apr 14, 2022
37a69cb
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Apr 14, 2022
39b3d32
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Apr 19, 2022
9c0439c
remove manifest.json
tomi-msft Apr 19, 2022
fd04253
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft May 5, 2022
53e77d3
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft May 9, 2022
afb2431
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft May 9, 2022
7507fae
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft May 13, 2022
3b6e5f9
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft May 20, 2022
b502589
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Jun 1, 2022
0d89f78
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Jun 9, 2022
66f1019
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Jun 21, 2022
6832725
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Jun 21, 2022
5327af7
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Jun 30, 2022
7b0bbb9
Release 1.1.179
fluentCI Aug 19, 2022
adab308
Merge branch 'master' of https://github.com/microsoft/fluentui-system…
tomi-msft Sep 22, 2022
10cf5fe
merge with main
tomi-msft Oct 24, 2022
3ec81c8
revert
tomi-msft Oct 24, 2022
74ea042
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Oct 31, 2022
934daf2
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Oct 31, 2022
71ecb5b
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Nov 14, 2022
772f374
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Jan 12, 2023
995334e
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Mar 7, 2023
b0573ab
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Mar 15, 2023
a1e8486
Add context to provide rtl support for react-icons
tomi-msft Mar 17, 2023
8fabcae
remove wrapIcon and consolidate logic
tomi-msft Mar 17, 2023
9da091e
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Mar 23, 2023
9e1d6e9
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Mar 30, 2023
07e1691
Add autoflipping logic to build script
tomi-msft Apr 3, 2023
3a10737
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Apr 3, 2023
dfdc77e
Merge branch 'main' of https://github.com/microsoft/fluentui-system-i…
tomi-msft Apr 18, 2023
12b991c
merge with main
tomi-msft Apr 20, 2023
f7f417f
incorporate changes to icon build into auto-flipping
tomi-msft Apr 21, 2023
1573195
merge
tomi-msft Apr 21, 2023
bc6674b
remove test filter script
tomi-msft Apr 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 12 additions & 5 deletions packages/react-icons/convert-font.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const SRC_PATH = argv.source;
const DEST_PATH = argv.dest;
// @ts-ignore
const CODEPOINT_DEST_PATH = argv.codepointDest;
// @ts-ignore
const RTL_FILTER_PATH =argv.filter;

if (!SRC_PATH) {
throw new Error("Icon source folder not specified by --source");
Expand All @@ -27,6 +29,9 @@ if (!DEST_PATH) {
if (!CODEPOINT_DEST_PATH) {
throw new Error("Output destination folder for codepoint map not specified by --dest");
}
if (!RTL_FILTER_PATH) {
throw new Error("Filter folder not specified by --filter");
}

processFiles(SRC_PATH, DEST_PATH)

Expand Down Expand Up @@ -62,7 +67,6 @@ async function processFiles(src, dest) {
const indexPath = path.join(dest, 'index.tsx')
// Finally add the interface definition and then write out the index.
indexContents.push('export { FluentIconsProps } from \'../utils/FluentIconsProps.types\'');
indexContents.push('export { default as wrapIcon } from \'../utils/wrapIcon\'');
indexContents.push('export { default as bundleIcon } from \'../utils/bundleIcon\'');
indexContents.push('export * from \'../utils/useIconState\'');
indexContents.push('export * from \'../utils/constants\'');
Expand All @@ -79,14 +83,16 @@ async function processFiles(src, dest) {
* @returns { Promise<string[]> } - chunked icon files to insert
*/
async function processFolder(srcPath, codepointMapDestFolder, resizable) {
const filterFile = fs.readFile(RTL_FILTER_PATH, { encoding: 'utf8' })
var rtlArray = (await filterFile).split(/\r?\n/);
var files = await glob(resizable ? 'FluentSystemIcons-Resizable.json' : 'FluentSystemIcons-{Filled,Regular}.json', { cwd: srcPath, absolute: true });

/** @type string[] */
const iconExports = [];
await Promise.all(files.map(async (srcFile, index) => {
/** @type {Record<string, number>} */
const iconEntries = JSON.parse(await fs.readFile(srcFile, 'utf8'));
iconExports.push(...generateReactIconEntries(iconEntries, resizable));
iconExports.push(...generateReactIconEntries(iconEntries, resizable, rtlArray));

return generateCodepointMapForWebpackPlugin(
path.resolve(codepointMapDestFolder, path.basename(srcFile)),
Expand Down Expand Up @@ -133,16 +139,17 @@ async function generateCodepointMapForWebpackPlugin(destPath, iconEntries, resiz
* @param {boolean} resizable
* @returns {string[]}
*/
function generateReactIconEntries(iconEntries, resizable) {
function generateReactIconEntries(iconEntries, resizable, rtlArray) {
/** @type {string[]} */
const iconExports = [];
var shouldAutoFlip;
for (const [iconName, codepoint] of Object.entries(iconEntries)) {
shouldAutoFlip = rtlArray.includes(iconName);
let destFilename = getReactIconNameFromGlyphName(iconName, resizable);

var jsCode = `export const ${destFilename} = /*#__PURE__*/createFluentFontIcon(${JSON.stringify(destFilename)
}, ${JSON.stringify(String.fromCodePoint(codepoint))
}, ${resizable ? 2 /* Resizable */ : /filled$/i.test(iconName) ? 0 /* Filled */ : 1 /* Regular */
}${resizable ? '' : `, ${/(?<=_)\d+(?=_filled|_regular)/.exec(iconName)[0]}`
}, ${shouldAutoFlip} ${resizable ? '' : `, ${/(?<=_)\d+(?=_filled|_regular)/.exec(iconName)[0]}`
});`;

iconExports.push(jsCode);
Expand Down
18 changes: 14 additions & 4 deletions packages/react-icons/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const _ = require("lodash");

const SRC_PATH = argv.source;
const DEST_PATH = argv.dest;
const RTL_FILTER_PATH =argv.filter;
Copy link
Collaborator

Choose a reason for hiding this comment

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

filter is a confusing name for this arg. How about flip_in_rtl?

Ditto for the file name: flip_in_rtl.txt instead of rtl.txt

Copy link
Member

Choose a reason for hiding this comment

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

Possibly even autoflip_in_rtl

const TSX_EXTENSION = '.tsx'

if (!SRC_PATH) {
Expand All @@ -16,11 +17,17 @@ if (!SRC_PATH) {
if (!DEST_PATH) {
throw new Error("Output destination folder not specified by --dest");
}
if (!RTL_FILTER_PATH) {
throw new Error("Filter folder not specified by --filter");
}

if (!fs.existsSync(DEST_PATH)) {
fs.mkdirSync(DEST_PATH);
}

const filterFile = fs.readFileSync(RTL_FILTER_PATH, { encoding: 'utf8' })
var rtlArray = filterFile.split(/\r?\n/);
//console.log(rtlArray)
processFiles(SRC_PATH, DEST_PATH)

function processFiles(src, dest) {
Expand Down Expand Up @@ -64,11 +71,12 @@ function processFiles(src, dest) {

const indexPath = path.join(dest, 'index.tsx')
// Finally add the interface definition and then write out the index.
indexContents.push('export { FluentIconsProps } from \'./utils/FluentIconsProps.types\'');
indexContents.push('export { default as wrapIcon } from \'./utils/wrapIcon\'');
indexContents.push('export type { FluentIconsProps } from \'./utils/FluentIconsProps.types\'');
indexContents.push('export { default as bundleIcon } from \'./utils/bundleIcon\'');
indexContents.push('export * from \'./utils/useIconState\'');
indexContents.push('export * from \'./utils/constants\'');
indexContents.push('export { IconDirectionContextProvider, useIconContext } from \'./contexts/index\'');
indexContents.push('export type { IconDirectionContextValue } from \'./contexts/index\'');

fs.writeFileSync(indexPath, indexContents.join('\n'), (err) => {
if (err) throw err;
Expand Down Expand Up @@ -100,7 +108,9 @@ function processFolder(srcPath, destPath, resizable) {
if(resizable && !file.includes("20")) {
return
}
var iconName = file.substr(0, file.length - 4) // strip '.svg'
// Check to see if the svg should be autoflipped
var shouldAutoFlip = rtlArray.includes(file);
var iconName = file.substring(0, file.length - 4) // strip '.svg'
iconName = iconName.replace("ic_fluent_", "") // strip ic_fluent_
iconName = resizable ? iconName.replace("20", "") : iconName
var destFilename = _.camelCase(iconName) // We want them to be camelCase, so access_time would become accessTime here
Expand All @@ -110,7 +120,7 @@ function processFolder(srcPath, destPath, resizable) {
const getAttr = (key) => [...iconContent.matchAll(`(?<= ${key}=)".+?"`)].map((v) => v[0]);
const width = resizable ? '"1em"' : getAttr("width")[0];
const paths = getAttr("d").join(',');
var jsCode = `export const ${destFilename} = (/*#__PURE__*/createFluentIcon('${destFilename}', ${width}, [${paths}]));`
var jsCode = `export const ${destFilename} = (/*#__PURE__*/createFluentIcon('${destFilename}', ${width}, [${paths}], ${shouldAutoFlip}));`
iconExports.push(jsCode);
}
});
Expand Down
5 changes: 2 additions & 3 deletions packages/react-icons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
"cleanSvg": "rm -rf ./intermediate",
"copy": "node ../../importer/generate.js --source=../../assets --dest=./intermediate --extension=svg --target=react",
"copy:font-files": "cpy './src/utils/fonts/*.{ttf,woff,woff2,json}' ./lib/utils/fonts/. && cpy './src/utils/fonts/*.{ttf,woff,woff2,json}' ./lib-cjs/utils/fonts/.",
"convert:svg": "node convert.js --source=./intermediate --dest=./src",
"convert:fonts": "node convert-font.js --source=./src/utils/fonts --dest=./src/fonts --codepointDest=./src/utils/fonts",
"convert:svg": "node convert.js --source=./intermediate --dest=./src --filter=../../importer/rtl.txt",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see rtl.txt included in this PR? If there is an existing file, it might be used for a different purpose? We probably want to create a new file for this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rtl.txt file is here.

I am wary about creating a new file for this because this file was already created by the designers to track icons that should be flipped in rtl

Copy link
Member

Choose a reason for hiding this comment

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

We need to track down with the Android folks (that's the only platform previously using the rtl.txt file) what is/isn't there. RTL wasn't kept up with, for the most part, as other platforms didn't need/use it.
We should also collaborate with the Apple folks on that, as they are the platform that has the most RTL/LTR support in, and it's supported by the platform itself, but we need to make sure everything has equivalency.

"convert:fonts": "node convert-font.js --source=./src/utils/fonts --dest=./src/fonts --codepointDest=./src/utils/fonts --filter=../../importer/rtl.txt",
"generate:font-regular": "node ../../importer/generateFont.js --source=intermediate --dest=src/utils/fonts --iconType=Regular --codepoints=../../fonts/FluentSystemIcons-Regular.json",
"generate:font-filled": "node ../../importer/generateFont.js --source=intermediate --dest=src/utils/fonts --iconType=Filled --codepoints=../../fonts/FluentSystemIcons-Filled.json",
"generate:font-resizable": "node ../../importer/generateFont.js --source=intermediate --dest=src/utils/fonts --iconType=Resizable",
Expand Down Expand Up @@ -81,4 +81,3 @@
}
}
}

13 changes: 13 additions & 0 deletions packages/react-icons/src/contexts/IconDirectionContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as React from 'react';

const IconDirectionContext = React.createContext<IconDirectionContextValue | undefined>(undefined);

export interface IconDirectionContextValue {
textDirection?: 'ltr' | 'rtl'
}

const IconDirectionContextDefaultValue: IconDirectionContextValue = {};

export const IconDirectionContextProvider = IconDirectionContext.Provider;

export const useIconContext = () => React.useContext(IconDirectionContext) ?? IconDirectionContextDefaultValue;
1 change: 1 addition & 0 deletions packages/react-icons/src/contexts/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './IconDirectionContext';
5 changes: 3 additions & 2 deletions packages/react-icons/src/utils/bundleIcon.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from "react";
import { iconFilledClassName, iconRegularClassName } from "./constants";
import type { FluentIcon } from "../utils/createFluentIcon";
import { FluentIconsProps } from "./FluentIconsProps.types";
import { makeStyles, mergeClasses } from "@griffel/react";

Expand All @@ -8,8 +9,8 @@ const useBundledIconStyles = makeStyles({
visible: { display: "inline" }
});

const bundleIcon = (FilledIcon: React.FC<FluentIconsProps>, RegularIcon: React.FC<FluentIconsProps>) => {
const Component: React.FC<FluentIconsProps> = (props) => {
const bundleIcon = (FilledIcon: FluentIcon, RegularIcon: FluentIcon) => {
const Component: FluentIcon = (props: FluentIconsProps) => {
const { className, primaryFill = 'currentColor', filled, ...rest } = props;
const styles = useBundledIconStyles();
return (
Expand Down
4 changes: 2 additions & 2 deletions packages/react-icons/src/utils/createFluentIcon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ export type FluentIcon = {
displayName?: string;
}

export const createFluentIcon = (displayName: string, width: string, paths: string[]): FluentIcon => {
export const createFluentIcon = (displayName: string, width: string, paths: string[], shouldAutoFlip: boolean): FluentIcon => {
const viewBoxWidth = width === "1em" ? "20" : width;
const Icon = (props: FluentIconsProps) => {
const state = {
...useIconState(props), // HTML attributes/props for things like accessibility can be passed in, and will be expanded on the svg object at the start of the object
...useIconState(props, shouldAutoFlip), // HTML attributes/props for things like accessibility can be passed in, and will be expanded on the svg object at the start of the object
width, height: width, viewBox: `0 0 ${viewBoxWidth} ${viewBoxWidth}`, xmlns: "http://www.w3.org/2000/svg"
};
return React.createElement(
Expand Down
4 changes: 2 additions & 2 deletions packages/react-icons/src/utils/fonts/createFluentFontIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ const useRootStyles = makeStyles({
},
});

export function createFluentFontIcon(displayName: string, codepoint: string, font: FontFile, fontSize?: number): React.FC<FluentIconsProps<React.HTMLAttributes<HTMLElement>>> & { codepoint: string} {
export function createFluentFontIcon(displayName: string, codepoint: string, font: FontFile, shouldAutoFlip: boolean, fontSize?: number): React.FC<FluentIconsProps<React.HTMLAttributes<HTMLElement>>> & { codepoint: string} {
const Component: React.FC<FluentIconsProps<React.HTMLAttributes<HTMLElement>>> & { codepoint: string} = (props) => {
useStaticStyles();
const styles = useRootStyles();
const className = mergeClasses(styles.root, styles[font], props.className);
const state = useIconState<React.HTMLAttributes<HTMLElement>>({...props, className});
const state = useIconState<React.HTMLAttributes<HTMLElement>>({...props, className}, shouldAutoFlip);


// We want to keep the same API surface as the SVG icons, so translate `primaryFill` to `color`
Expand Down
16 changes: 13 additions & 3 deletions packages/react-icons/src/utils/useIconState.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { FluentIconsProps } from "./FluentIconsProps.types";
import { makeStyles, mergeClasses } from "@griffel/react";
import { useIconContext } from "../contexts";

const useRootStyles = makeStyles({
root: {
Expand All @@ -9,21 +10,30 @@ const useRootStyles = makeStyles({
"@media (forced-colors: active)": {
forcedColorAdjust: 'auto',
}
},
flipped: {
transform: 'scaleX(-1)'
}
});

export const useIconState = <TBaseAttributes extends (React.SVGAttributes<SVGElement> | React.HTMLAttributes<HTMLElement>) = React.SVGAttributes<SVGElement>>(props: FluentIconsProps<TBaseAttributes>): Omit<FluentIconsProps<TBaseAttributes>, 'primaryFill'> => {
export const useIconState = <TBaseAttributes extends (React.SVGAttributes<SVGElement> | React.HTMLAttributes<HTMLElement>) = React.SVGAttributes<SVGElement>>(props: FluentIconsProps<TBaseAttributes>, shouldAutoFlip: boolean): Omit<FluentIconsProps<TBaseAttributes>, 'primaryFill'> => {
const { title, primaryFill = "currentColor", ...rest } = props;

tomi-msft marked this conversation as resolved.
Show resolved Hide resolved
const state = {
...rest,
title: undefined,
fill: primaryFill
} as Omit<FluentIconsProps<TBaseAttributes>, 'primaryFill'>;

const iconContext = useIconContext();
const styles = useRootStyles();

state.className = mergeClasses(styles.root, state.className);

if(shouldAutoFlip) {
state.className = mergeClasses(styles.root, iconContext.textDirection === 'rtl' && styles.flipped, state.className);
} else {
state.className = mergeClasses(styles.root, state.className);
}

if (title) {
state['aria-label'] = title;
}
Expand Down
14 changes: 0 additions & 14 deletions packages/react-icons/src/utils/wrapIcon.tsx

This file was deleted.