chore: V4 cherry pick #8910
chore: V4 cherry pick #8910
Conversation
…y#8708) This by large avoids importing from the high level export file from @patternfly/react-core as that exports the whole patternfly react exported modules, which affects consumers if tree-shaking algorithm does not mark them as dead code. Noticed these problematic imports while trying to port out project to 'esbuild' which created a noticably big dist'ed CSS because of these. [1] [1] evanw/esbuild#2933
* fix(charts) ensure ChartAxis labels, as well as tick labels, are assigned unique IDs. Closes patternfly#8731 * chore(charts) fixed typo in documentation * chore(charts) remove comma for linter
* fix(DatePicker): fixed errors in demos * Updated error message in demos
* fix(TimePicker): called onChange when pressing Enter * Added unit test * Updated unit test to ensure time suffix is present in onChange
* feat(Table): add right sticky column support * Updated prop and example verbiage --------- Co-authored-by: Eric Olkowski <thatblindgeye@gmail.com>
|
Preview: https://patternfly-react-pr-8910.surge.sh A11y report: https://patternfly-react-pr-8910-a11y.surge.sh |
kmcfaul
left a comment
kmcfaul
left a comment
There was a problem hiding this comment.
Minor note about deprecated Dropdown usage but not a blocker. Everything else looks good to me
| import { SortByDirection } from './SortColumn'; | ||
| import { DropdownDirection, DropdownPosition } from '@patternfly/react-core/dist/esm/deprecated/components'; | ||
| import { DropdownItemProps } from '@patternfly/react-core/dist/esm/components/Dropdown'; | ||
| import { DropdownDirection, DropdownPosition } from '@patternfly/react-core/dist/esm/deprecated/components/Dropdown'; |
There was a problem hiding this comment.
We may want to make a note to clean up the remaining usages of the old dropdown. Looks like it's still referenced in IColumn here and TdActionsType in base/types.
There was a problem hiding this comment.
@thatblindgeye is this part of your PR for updating demos to use the new dropdown?
There was a problem hiding this comment.
Yeah that's something that I should be able to incorporate into #8839
There was a problem hiding this comment.
These updates are mostly looking good! I'm just seeing some bugs in our sticky column examples that may have snuck in with these earlier commits; the sticky column demos that should be left-aligned aren't working. Do you think we should open a separate issue @tlabaj ?
| ```ts file="TableMultipleStickyColumns.tsx" | ||
| ``` | ||
|
|
||
| ### Composable: Multiple right-aligned sticky columns |
There was a problem hiding this comment.
For consistency
| ### Composable: Multiple right-aligned sticky columns | |
| ### Multiple right-aligned sticky columns |
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
Other than Jenny's comment about the sticky example, I also just had the below comment but otherwise things look good
| }); | ||
| }); | ||
|
|
||
| test('should call onChange when pressing Enter', async () => { |
There was a problem hiding this comment.
Would we want to skip this test for and update it as a followup?
There was a problem hiding this comment.
what is the issue so I can note it in my PR.
There was a problem hiding this comment.
It's just expecting the wrong time on GitHub for some reason. I pulled your branch locally and the test passes fine there, so just another time related issue with tests
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
6 participants
What: Closes #8911
Pulled in change from the last v4 major release.