In my last post, I shared my experience about my first pull request to ant-design. In this post, I would like to continue writing about my second PR. Hope it's helpful to you.
Issue
The issue was opened by li-jia-nan. This issue was about the component Affix
. Users can wrap Affix
around another component to make it stick the viewport. And it supports setting a target element:
The issue was that it didn't work as expected after some changes, which means that when the user scrolls up, the blue button would not fixed at the top of container, See:
Debug process
I spent lots of time checking the source code ant-design/components/affix/index.tsx
. What I did was adding lots of logs of each function to familiar with the logic of calling relations. Although it was a bit boring and frustrating before finding the root cause, thanks to the help of logs, I finally realized that the logic of this code should be fixed:
Before:
const addListeners = () => {
const listenerTarget = targetFunc?.();
TRIGGER_EVENTS.forEach((eventName) => {
if (prevListener.current) {
prevTarget.current?.removeEventListener(eventName, prevListener.current);
}
listenerTarget?.addEventListener(eventName, lazyUpdatePosition);
});
prevTarget.current = listenerTarget;
prevListener.current = lazyUpdatePosition;
};
After:
const addListeners = () => {
const listenerTarget = targetFunc?.();
// added
if (!listenerTarget) {
return;
}
TRIGGER_EVENTS.forEach((eventName) => {
if (prevListener.current) {
prevTarget.current?.removeEventListener(eventName, prevListener.current);
}
listenerTarget?.addEventListener(eventName, lazyUpdatePosition);
});
prevTarget.current = listenerTarget;
prevListener.current = lazyUpdatePosition;
};
In before code, when listenerTarget
is null
, the prevTarget
can still do removeEventListener
, so the prevTarget
element will not listen to the scroll event anymore. This sounds a bit funny: there was no scrolling EventListener, yet the button started scrolling instead. In fact, that EventListener is used to monitor the relative position and add CSS position: fixed
to make this button fixed. What I did was add a check on the listenerTarget
, if it is null
, the function addListeners
just returns, so the prevTarget
remains unaffected.
Reflection
The key to fixing this issue is patience, print functions' passing values little by little to understand the whole picture. I admit that this approach might be a bit clumsy, but it's still a way of tackling the problem, isn't it? After all, in the world of coding, sometimes a little clumsiness is just the first step towards mastering a new skill.
Top comments (0)