iTwin / iTwinUI-react

A react component library for iTwinUI.
https://github.com/iTwin/iTwinUI
Other
84 stars 23 forks source link

chore: Updated itwinui-css to dev.1 #796

Closed veekeys closed 2 years ago

veekeys commented 2 years ago
mayank99 commented 2 years ago

@veekeys Did you go through all the images yourself before approving?

I noticed that the Table paginator is missing active state.

Same thing in sidenav

And the expandable block status icons are missing fill


Also, the Tree images' height seems to have gone down but this might be an issue with how cypress is capturing the screenshot

Also, the Slider icon labels have become more dark, but I think that's fixed in my variables PR.

veekeys commented 2 years ago

@veekeys Did you go through all the images yourself before approving?

I noticed that the Table paginator is missing active state.

Same thing in sidenav

And the expandable block status icons are missing fill

Also, the Tree images' height seems to have gone down but this might be an issue with how cypress is capturing the screenshot

Also, the Slider icon labels have become more dark, but I think that's fixed in my variables PR.

I did it only locally with really bad Cypress html file by scrolling through... It took me a while to actually run all the tests, cause they all were failing.. I will do it through github and image comparer deeper, but your feedback is already really helpful. Thank you

veekeys commented 2 years ago

Oh wow... I have updated Paginator, SideNav, Tile and ExpandableBlock which were ALL MISSED either from Icon changes or Button changes! This is huge miss! I will need to verify Slider and Tree is interesting too... Apart from these, I have noticed some other changes, which might be worth to raise, I will try to put those in a list tomorrow.

veekeys commented 2 years ago

Ok, I checked situation in css, so slider icons are fixed as you mentioned with the css variables PR. So here we should be good for now.

r100-stack commented 2 years ago

Some test image diffs I found:

gretanausedaite commented 2 years ago

Some test image diffs I found:

I think we should keep current header test image with all not disabled buttons to keep it as it is currently. Edit: nvm, I was wrong. In css v1 changes we have disabled header button. Keeping it this way.

Everything else looked fine to me. Will check again