Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(docs): fix conflict Fela and FontAwesome #951

Merged
layershifter merged 5 commits into from
Feb 22, 2019
Merged

fix(docs): fix conflict Fela and FontAwesome #951

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
7628244
fix(docs): fix conflict Fela and FontAwesome
layershifter Feb 22, 2019
08c4a06
add changelog entry
layershifter Feb 22, 2019
0cd373c
fix danger!
layershifter Feb 22, 2019
7a15c0b
use default too
layershifter Feb 22, 2019
c9a189f
Merge branches 'docs/fela-fa' and 'master' of https://github.com/star…
layershifter Feb 22, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Display correctly images in portrait mode inside `Avatar` @layershifter ([#899](https://github.com/stardust-ui/react/pull/899))
- Expose `Popup`'s content Ref @sophieH29 ([#913](https://github.com/stardust-ui/react/pull/913))
- Fix `Button` Teams theme styles to use semibold weight @notandrew ([#829](https://github.com/stardust-ui/react/pull/829))
- Fix conflicts of generated names in Fela with FontAwesome @layershifter ([#951](https://github.com/stardust-ui/react/pull/951))

### Documentation
- Add `Editable Area with Dropdown` prototype for mentioning people using `@` character (only available in development mode) @Bugaa92 ([#931](https://github.com/stardust-ui/react/pull/931))
Expand Down
2 changes: 1 addition & 1 deletion dangerfile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const hasAddedLinesAfterVersionInChangelog = async (): Promise<boolean> => {
return acc.concat(filteredLines)
}, [])

return addedLines.some(line => line.ln >= versionLineNumber)
return addedLines.some(line => line.ln > versionLineNumber)
}

async function getChangedDependencies(filepath, dependenciesKey = 'dependencies') {
Expand Down
8 changes: 8 additions & 0 deletions packages/react/src/lib/felaRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ import rtl from 'fela-plugin-rtl'

import { Renderer } from '../themes/types'

// Blacklist contains a list of classNames that are used by FontAwesome
// https://fontawesome.com/how-to-use/on-the-web/referencing-icons/basic-use
const blacklistedClassNames = ['fa', 'fas', 'far', 'fal', 'fab']
Copy link
Collaborator

@bmdalex bmdalex Feb 22, 2019

Choose a reason for hiding this comment

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

a bit concerned about these as they can change at any time in fontawesome and we have no control over that..

Copy link
Contributor

@kuzhelov kuzhelov Feb 24, 2019

Choose a reason for hiding this comment

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

agree - especially given that each theme might load CSS libraries with different set of class names that should be blacklisted. Would expect that this blacklisted information will be defined by theme, not (only) general Stardust logic - and, thus, would be extendable


const filterClassName = (className: string): boolean =>
className.indexOf('ad') === -1 && blacklistedClassNames.indexOf(className) === -1

const createRendererConfig = (options: any = {}) => ({
devMode: process.env.NODE_ENV !== 'production',
plugins: [
Expand All @@ -24,6 +31,7 @@ const createRendererConfig = (options: any = {}) => ({
felaPluginFallbackValue(),
...(options.isRtl ? [rtl()] : []),
],
filterClassName,
Copy link
Member Author

@layershifter layershifter Feb 22, 2019

Choose a reason for hiding this comment

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

https://github.com/rofrischmann/fela/blob/v10.2.1/packages/fela/src/generateClassName.js#L25

Copy link

@robinweser robinweser Feb 22, 2019

Choose a reason for hiding this comment

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

Just as I see it here: you can now use the rtl plugin with a theme.direction property to trigger rtl transformation (check the plugin docs) that way you dont have to conditionally add the plugin

Copy link
Member Author

@layershifter layershifter Feb 22, 2019

Choose a reason for hiding this comment

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

Yep, seen these changes. Going to visit this place in a separate PR 👍

enhancers: [],
...(options.isRtl ? { selectorPrefix: 'rtl_' } : {}),
...(options.rendererId ? { rendererId: options.rendererId } : {}),
Expand Down