yardz commented on a change in pull request #13220:
URL: https://github.com/apache/superset/pull/13220#discussion_r579203541
##########
File path: superset-frontend/src/explore/components/ExploreViewContainer.jsx
##########
@@ -146,11 +147,11 @@ const getWindowSize = () => ({
function useWindowSize({ delayMs = 250 } = {}) {
const [size, setSize] = useState(getWindowSize());
- useEffect(() => {
+ useComponentDidMount(() => {
const onWindowResize = debounce(() => setSize(getWindowSize()), delayMs);
window.addEventListener('resize', onWindowResize);
return () => window.removeEventListener('resize', onWindowResize);
- }, []);
+ });
Review comment:
Let's do this way. I will revert this commit where I put this hook in
these other components. I didn't really read them, I just saw that they was
using it that way and switched to hook... I just made this change to avoid
repeated code. As it has already led to all this discussion, it has lost its
purpose.
The hook is ok, the component is ok. It does not make sense to keep blocking
the merge due to other components that are not the scope of this PR. Do you
agree with that?
----------------------------------------------------------------
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]
