feat(Table): added optin animations for expansion #11865
Conversation
|
Preview: https://patternfly-react-pr-11865.surge.sh A11y report: https://patternfly-react-pr-11865-a11y.surge.sh |
thatblindgeye
requested review from
a team,
kaylachumley,
lboehling,
rebeccaalpert and
wise-king-sullyman
and removed request for
a team
rebeccaalpert
left a comment
rebeccaalpert
left a comment
There was a problem hiding this comment.
This looks fine overall in my opinion. Code looks good. Examples seem to work. I did see a typo in a prop description - otherwise happy to approve.
This is likely not you, but did also want to call out a difference I noticed between here and the main site with spacing, similar to what we saw in a different PR.
| Old | New |
|---|---|
 |
 |
 |
 |
|
cc @mcoker for Rebecca's comment above regarding spacing discrepancy |
srambach
left a comment
srambach
left a comment
There was a problem hiding this comment.
I was trying to figure out how we might smooth out the compound expandable, but when I changed it so they aren't all the same height, it feels far less glitchy. I say go ahead and let's keep an eye on real world use and feedback.
thatblindgeye
force-pushed
the
iss11866_tableExpandAnims
branch
2 times, most recently
from
6606ef1 to
ff6e5e0
Compare
kaylachumley
left a comment
kaylachumley
left a comment
There was a problem hiding this comment.
the expansion itself seems good! but the blue ripple color on the expansion button feels distracting for me... side thought: wondering if we can apply a grey ripple color instead for expansion?
|
@thatblindgeye ope sorry! realized i was reviewing the wrong component! the comment above was for the expandable section which may not even be representative of what we offer since its a surge link for the wrong component review |
thatblindgeye
force-pushed
the
iss11866_tableExpandAnims
branch
from
ff6e5e0 to
378168e
Compare
nicolethoen
left a comment
nicolethoen
left a comment
There was a problem hiding this comment.
I dont think the compound expandable examples should have these animations?
|
@nicolethoen that might be a visual issue we've noticed where both the previous expanded thing and the new expanded thing are both momentarily rendered. Could you slow the animations down to 10% in dev tools to confirm if that's what's happening there? If it is then it's a known visual issue, but just to confirm @mcoker we're still good to merge and work on a potential fix for that? |
|
I can take a look - I didn't expect the table rows to collapse as I switched between different tabs |
|
|
thatblindgeye
force-pushed
the
iss11866_tableExpandAnims
branch
from
378168e to
c46b0b3
Compare
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
7 participants
What: Closes #11856
Additional issues: