fix(styling): Fixed styling for props table headings #66
Conversation
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
This lgtm, only thing is the large amount of lines changed in the yarn lock file. Can you just double check whether the yarn lock needs to be updated to that degree? Wondering if maybe a yarn install call locally would've caused that size of an update.
Also @wise-king-sullyman any issues with bumping the react-core version? Not sure if we wanted to stay on 6.0.0 for any particular reason.
wise-king-sullyman
left a comment
wise-king-sullyman
left a comment
There was a problem hiding this comment.
Also @wise-king-sullyman any issues with bumping the react-core version? Not sure if we wanted to stay on 6.0.0 for any particular reason.
I don't think there should be any issues, but I do wonder what prompted it?
Also I do think we should probably bump all the PF packages at once rather than just react-core if we are going to bump it.
Change request primarily because we shouldn't be adding a yarn.lock, we actually just use npm in this repo not yarn.
Also @thatblindgeye your issue also mentioned adding a heading before the table, is that something you still want to see as part of this PR?
|
@wise-king-sullyman yeah that should be a quick enough change that it makes sense to include here. Did we want to just keep the "Props" heading name for that section for now? I think you and I had talked about that and whether something else made sense as the heading or not, but can't 100% recall what that may have been. |
package-lock.json
Outdated
| "@patternfly/patternfly": "^6.0.0", | ||
| "@patternfly/react-code-editor": "^6.2.2", | ||
| "@patternfly/react-core": "^6.0.0", | ||
| "@patternfly/react-core": "^6.2.2", |
There was a problem hiding this comment.
Looks like you need to update the lock file, you can do that by running npm i
src/components/PropsTable.tsx
Outdated
| import { css } from '@patternfly/react-styles' | ||
| import accessibleStyles from '@patternfly/react-styles/css/utilities/Accessibility/accessibility' | ||
| import textStyles from '@patternfly/react-styles/css/utilities/Text/text' | ||
| import {Content} from '@patternfly/react-core'; |
There was a problem hiding this comment.
Can you run the formatter on this file?
src/components/PropsTables.astro
Outdated
| {propsData | ||
| .filter((comp: any) => !!comp) | ||
| .map((component: any) => ( | ||
| <StackItem> |
There was a problem hiding this comment.
You'll need to move the key from the PropsTable component up to this StackItem now that it's the top level component being returned by the map.
There was a problem hiding this comment.
Looks like you beat me to this one!
It doesn't hurt anything, but you can remove the key from the PropsTable still.
|
🎉 This PR is included in version 1.8.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Fixed the styling for the props table headings, by utilizing the
<Content>tag, ensuring that the styling was not being overridden.Closes #58