material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

Published 20 hours ago • verified publisher icon mui

[docs] Remove empty tags on the TransferList demos

Open ekusiadadus opened this issue 3 years ago • 9 comments

Why

I found a JSX closing tag error in docs about TransferList and fixed it. https://github.com/mui/material-ui/blob/60eaa3bf41cf89b796e74eea0f3497d0e02ec959/docs/data/material/components/transfer-list/TransferList.tsx#L89

ekusiadadus avatar Jun 13 '22 04:06 ekusiadadus

No bundle size changes

Generated by :no_entry_sign: dangerJS against e9d4a8f443df275e1494ae6969cbe5507a0ef520

mui-bot avatar Jun 13 '22 04:06 mui-bot

I can see the same in the SelectAllTrasferList.jsx|tsx. Would you mine updating those too?

mnajdova avatar Jun 22 '22 12:06 mnajdova

@mnajdova Thanks for your reviewing.

I have also fixed SelectAllTransferList.

ekusiadadus avatar Jun 23 '22 04:06 ekusiadadus

@mnajdova

I checked it again and I still think it contains unnecessary tags. <- it's not broken, but unnecessary.

The demo did not appear to be broken either.

Could you to check it again?

https://github.com/mui/material-ui/blob/2951e82a128199eecb317938d0af4582d3536e2a/docs/data/material/components/transfer-list/TransferList.tsx#L89

ekusiadadus avatar Nov 02 '22 07:11 ekusiadadus

@mnajdova

I checked it again and I still think it contains unnecessary tags. <- it's not broken, but unnecessary.

The demo did not appear to be broken either.

Could you to check it again?

The demo is broken in https://deploy-preview-33127--material-ui.netlify.app/material-ui/react-transfer-list/#basic-transfer-list, for example try to move the first list item to the right.

mnajdova avatar Nov 10 '22 13:11 mnajdova

@mnajdova

I checked it once again, and recorded it... I didn't think it was broken.

A self closing tag on this line is not necessary...

https://user-images.githubusercontent.com/70436490/201117638-7668f5ec-4e9f-4662-8478-0bd4c61ff136.mp4

ekusiadadus avatar Nov 10 '22 14:11 ekusiadadus

@ekusiadadus I was refering to the example in the PR: https://deploy-preview-33127--material-ui.netlify.app/material-ui/react-transfer-list/#basic-transfer-list I can't see if the code on your side is changed, but it is clearly broken on the PR preview:

transfer-list-bug

mnajdova avatar Dec 13 '22 14:12 mnajdova

@mnajdova

Thanks for your review and sample movie. But, still I'm not sure it's broken....

You clicked 'Transfer All Item' button, is it right? Then the code works correct.

If you want to transfer only selected items, please click the button below.

I've checked my code and preview again, and still I think this code works correct.

ekusiadadus avatar Dec 13 '22 23:12 ekusiadadus

You clicked 'Transfer All Item' button, is it right? Then the code works correct.

🤦‍♀️ you are right, I kept pressing the Trasfer all button for some reason.

mnajdova avatar Dec 14 '22 14:12 mnajdova