feat: remove fonts from package, update ActionBar, Button and InfoLabel #908
Conversation
jbadan
changed the title
| <style type="text/css"> | ||
| @font-face { | ||
| font-family: '72'; | ||
| src: url('./72-Regular.woff') format('woff'); |
There was a problem hiding this comment.
This is stupid and annoying, but I couldn't get the fonts from @sap-theming to load from node_modules in here 😢
|
Deploy preview for fundamental-react ready! Built with commit 508f3a3 |
| fs.copyFile('README.md', 'src/_playground/documentation/Home/README.md', (err) => { | ||
| if (err) throw err; | ||
| }); | ||
|
|
There was a problem hiding this comment.
moved to scripts/copy-assets.js
| buttonContainerClassNames: 'Classnames to spread to the back Button container.', | ||
| description: 'Localized text for the description.', | ||
| descriptionProps: 'Additional props to be spread to the description\'s `<p>` element.', | ||
| headingLevel: 'Heading level. `<h1>` is reserved for the page title.', |
There was a problem hiding this comment.
if h1 is reserved, should the propType be CustomPropTypes.range(2, 6), then?
src/ActionBar/ActionBar.js
Outdated
| actionClassNames: 'Classnames to spread to the action Button container.', | ||
| actionProps: 'Props to spread to the action Button container', | ||
| actions: 'Button components to add to the ActionBar.', | ||
| buttonContainerClassNames: 'Classnames to spread to the back Button container.', |
There was a problem hiding this comment.
Seems weird to have actionClassNames and buttonContainerClassNames plural when we use className which isn't plural for the same idea.
There was a problem hiding this comment.
Looks like there is a mixture of both ways of doing it in fundamental-react
actionClassNames
buttonContainerClassNames
labelClassNames
inputClassNames
listClassNames
tokenizerClassNames
vs
backdropClassName
contentClassName
inputClassName
popperClassName
tableBodyClassName
table....ClassName
referenceClassName
There was a problem hiding this comment.
I think that came from a bunch of different people implementing it. We should do a separate story to clean it up in one go.
I'll make this one singular for now.
There was a problem hiding this comment.
Now that I look at the history on it.... it's only me that's been adding the plural ones 😆🤣
src/ActionBar/ActionBar.js
Outdated
| description: 'Localized text for the description.', | ||
| descriptionProps: 'Additional props to be spread to the description\'s `<p>` element.', | ||
| headingLevel: 'Heading level. `<h1>` is reserved for the page title.', | ||
| onClick: 'Callback to pass to the back Button container.' |
There was a problem hiding this comment.
isn't it passed to the button and not its container?
There was a problem hiding this comment.
There is no subcomponent anymore. That said, should we name this prop something to reflect that it's for the back button (e.g. onBackClick)?
package.json
Outdated
| "start:playground": "npm run storybook", | ||
| "std-version": "standard-version -m \"chore(release): version %s build ${TRAVIS_BUILD_NUMBER} [ci skip]\"", | ||
| "storybook": "start-storybook -p 12123", | ||
| "storybook": "node scripts/copy-assets.js && start-storybook -p 12123 -s src/_playground/fonts", |
There was a problem hiding this comment.
Would we want to make the static directory more generic and then just include a fonts directory within it?
src/ActionBar/ActionBar.js
Outdated
| description: 'Localized text for the description.', | ||
| descriptionProps: 'Additional props to be spread to the description\'s `<p>` element.', | ||
| headingLevel: 'Heading level. `<h1>` is reserved for the page title.', | ||
| onClick: 'Callback to pass to the back Button container.' |
There was a problem hiding this comment.
There is no subcomponent anymore. That said, should we name this prop something to reflect that it's for the back button (e.g. onBackClick)?
Description
Badgefolder toCounterfolder - import asimport { Counter } from 'fundamental-react/Counter'option=‘light’tooption=‘transparent’