yardz commented on pull request #13458:
URL: https://github.com/apache/superset/pull/13458#issuecomment-808459036


   > @geido I would say that when you look at the component's name, you can 
imagine a reusable component used in multiple places. But when you look at the 
implementation and props you will see that is just a div with almost no 
behavior and used only in `TableElement`. I have difficulty imagining creating 
tests and a storybook for this at its current stage. Maybe in the future, if a 
`Fade` component is required in other places and with more behavior, we can 
move it to `src/components`.
   
   I have a very strong opinion about not assembling components (to be honest i 
will always be against it).
   I did a very quick and superficial search and saw several components using 
"some kind" of fade... Which brings me to the question, why this fade is not 
being used in more places?
   I believe that a piece of the reason is precisely because there is no fade 
in `src/components ` and the idea that "if a component is used only here, I 
will put it here".
   
   But I don't see anything wrong with the code. I just disagree with the PR's 
goal.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to