No backing away when hacking away 🎃🚀

My last blog post focused on a pull request in a small to medium sized project, but the contributions were of significant impact.

This week, it was time to level up! While I was looking for a bigger and more complex project for my 4th and final Hacktoberfest Pull Request, I came across my professor's project that he had shared on our class' Slack Channel.

Table of Contents

 1. The Project 💬
 2. The Issue - Scrolling Bug 🐞
 3. Fixing the issue
 4. The Pull Request ⚡️
       4.1. The Actual Fix 💡
 5. Conclusion 🎊
 6. Bonus PR 🥷

The Project 💬

ChatCraft.org is a developer-oriented ChatGpt clone allowing the user to effortlessly switch between various language models.

The UI was very well designed, using different styles and even implementing different versions of some components to respect various screen sizes.

A quick demo image from the official README:

I also found a couple of blog posts from the authors sharing valuable insights about the project and its development phases.

• https://blog.humphd.org/chatcraft-org/
• https://taras.glek.net/post/chatcraft.org-opensourcechatgpt-for-devs/

While reading the first blog, I found a part that left me laughing hard as I could completely relate to it.

When I'd get stumped on something, I paste in the code and start talking about my bugs. Because I work in Markdown, it's very similar to writing issues on GitHub. Often I get what I need back: a push in the right direction and sometimes complete code as well. I've also been amazed at how it has been able to replace automate tests. For example, the other day I was working on a bug in the syntax highlighting code, and I worked with ChatCraft on ChatCraft. I'd ask it for examples of code blocks, fix the code, repeat, ask about bugs I was seeing, fix things, repeat. Using the app as an AI-REPL is extremely productive and unlike any programming I've done before. It's like assembling a robot with the robot's help.

Exactly same happened with me while working on the bug I'll be talking about in this blog. I've also recently gotten used to this approach of talking to ChatGpt when debugging problems and discussing about my code. Its like a modern version of the classic rubber duck debugging that we only hear about in memes these days. Its hard to describe the amusing feeling I got while talking to the application I was trying to fix about how to fix it 😆.

The Issue - Scrolling Bug 🐞

Now that we already have the context of what the project is about, its time to talk about the issue.

The problem was, whenever someone clicked on one of the sidebar links to mount a saved chat, or any other links requiring the window to scroll up, it broke the UI giving an impression that the entire viewport is being shifted up.

Fixing the issue

When talking about the issue, I was suggested that it could be something related to react-router-dom, so I started my debugging process from there.

Since I had only dealt with default routing provided by next.js before for a React project, I had to go through the documentation for react-router-dom to make sure any routing logic was not causing the problem.

This was the meat of entire app routing,

// Loading a shared chat remotely as JSON, which will be readonly
  {
    path: "/c/:user/:chatId",
    async loader({ params }) {
      const { user, chatId } = params;
      if (!(user && chatId)) {
        return redirect("/");
      }

      try {
        return loadShare(user, chatId);
      } catch (err) {
        console.warn(`Error loading shared chat ${user}/${chatId}`, err);
        redirect(`/`);
      }
    },
    element: <RemoteChat />,
    errorElement: <AppError />,
  },

And after an hour of analysis, I decided to give a clean chit to routing logic. It was time to look into the actual component this route loaded.

// Load a chat from over the network as a JSON blob (already available via loader)
export default function RemoteChat() {
  const chat = useLoaderData() as ChatCraftChat;

  return chat ? (
    <AutoScrollProvider>
      <ChatBase chat={chat} />
    </AutoScrollProvider>
  ) : null;
}

To be honest, this bug was really tricky to fix as there were endless pieces of code literally screaming I am the culprit, since lots of scrolling logic had to be implemented in order to make autoscrolling work during response generation. It reminded me of a really fun game from childhood called wink murder where you had to figure out who was the killer, while everyone tried to trick you into thinking they were the one.

Anyways, next I examined the AutoScrollProvider

export const useAutoScroll = () => useContext(AutoScrollContext);

export const AutoScrollProvider: FC<{ children: ReactNode }> = ({ children }) => {
  const [scrollProgress, setScrollProgress] = useState(0);
  const [shouldAutoScroll, setShouldAutoScroll] = useState(false);
  const scrollBottomRef = useRef<HTMLDivElement>(null);

  const value = {
    scrollProgress,
    incrementScrollProgress() {
      setScrollProgress((currentValue) => currentValue + 1);
    },
    resetScrollProgress() {
      setScrollProgress(0);
    },
    scrollBottomRef,
    shouldAutoScroll,
    setShouldAutoScroll,
  };
  return <AutoScrollContext.Provider value={value}>{children}</AutoScrollContext.Provider>;
};

but didn't find it to have any links to our issue.

Now it was time to look at the ChatBase component that was using the Grid from Chakra UI to layout all the elements.

return (
    <Grid
      w="100%"
      h="100%"
      gridTemplateRows="min-content 1fr min-content"
      gridTemplateColumns={{
        base: isSidebarVisible ? "300px 1fr" : "0 1fr",
        sm: isSidebarVisible ? "300px 1fr" : "0 1fr",
        md: isSidebarVisible ? "minmax(300px, 1fr) 4fr" : "0: 1fr",
      }}
      bgGradient="linear(to-b, white, gray.100)"
      _dark={{ bgGradient: "linear(to-b, gray.600, gray.700)" }}
    >
      <GridItem colSpan={2}>
        <Header
          chatId={chat.id}
          inputPromptRef={inputPromptRef}
          onToggleSidebar={handleToggleSidebarVisible}
        />
      </GridItem>
      ...
      ...
      <GridItem>
        <Box maxW="900px" mx="auto" h="100%">
          {chat.readonly ? (
            <Flex w="100%" h="45px" justify="end" align="center" p={2}>
              <NewButton forkUrl={`./fork`} variant="solid" />
            </Flex>
          ) : (
            <PromptForm
              forkUrl={`./fork`}
              onSendClick={onPrompt}
              isLoading={loading}
              previousMessage={chat.messages().at(-1)?.text}
              inputPromptRef={inputPromptRef}
            />
          )}
        </Box>
      </GridItem>
    </Grid>

I started suspecting there could be something wrong in the grid properties, so began tweaking the styles in the browser dev tools.

Soon, I figured out there was a lot of extra space inside the main element of the page likely due to some overflow. It was hidden from the user because of overflow: hidden style.

As soon as I disabled that style, a second scrollbar popped up revealing the extra space.

This gave away the first useful hint towards my debugging process. Whenever those chat links were clicked, they tried to scroll the window such that they were at the top of the page.

This behavior not only scrolled the part that was visible, but the entire window including the overflowing part that was hidden from the user.

Example:

Since there was no scrollbar to scroll back up with the default styling of overflow:hidden, the screen was permanently broken until the page was refreshed.

Now that I knew the problem, it was time to add the fix and open the pull request.

The Pull Request ⚡️

My first solution was rather naive, but it fixed the issue partially.

I added a dummy anchor element at the top of the Grid, and called the scrollIntoView method on it whenever the ChatBase component was rendered.

You can see the code here:
https://github.com/tarasglek/chatcraft.org/pull/271/commits/d38e34894544d5af295c3f52babc246a2514ba82

Now I knew this was not a good solution, and there were bound to be related issues.

I got a reply from my professor pointing out how the problem still persisted.

This was no surprise, so I decied to go after the root cause this time.

The Actual Fix 💡

I had a feeling that it was one of the items in the grid itself, that was causing the overflow. So I tried to find that problematic item by rebuilding the entire grid piece by piece. Everything was fine until I put in the PromptForm.

Digging deeper and deeper, I found that the Menu for selecting different chat models was the culprit.

It tried to create space so it could open downwards, hence adding all the extra height to the viewport.

Now that the actual culprit was busted, it was time to do it justice. I quickly opened up the component's documentation, and found some really useful props.

And... ...
you guessed it, I found the fix.

Making the following adjustments removed the overflowing space from the main element.

1. Using position fixed instead of absolute.
2. Fixing the height of chat client menu and adding a scrollbar.
3. Changing the sidebar rowSpan from 3 to 2, as only 2 more rows are available (optional).

Since there was no extra space, there was no room for strange scrolls that blew up the UI.

Here come the commits:

You'll notice I made sure to fix the mobile view as well, as I had noticed early on there were different versions for it.

MobilePromptSendButton

return (
    <ButtonGroup variant="outline" isAttached>
      <Menu placement="top" strategy="fixed"> // Additions here
        <IconButton
          type="submit"
          size="lg"
          variant="solid"
          isRound
          aria-label="Submit"
          isLoading={isLoading}
          icon={<TbSend />}
        />
        ...
        ...
        <MenuList height={400} overflowY={"auto"}> // and here
          {models.map((model) => (
            <MenuItem key={model.id} onClick={() => setSettings({ ...settings, model })}>
              {model.prettyModel}
            </MenuItem>
          ))}
        </MenuList>
      </Menu>
    </ButtonGroup>
  );

Conclusion 🎊

And that is how I went from understanding the project structure, the partial cause of the problem, the actual culprit, to the real fix that could be safely integrated into such a high quality codebase.

I have to say, even though my changes were small in the PR, but it was the most exciting one to work on as it was full of challenges, uncertainty, and a satisfying feeling after all that brainstorming paid off.

Bonus PR 🥷

While I was waiting for my latest changes to be reviewed, I contributed to another repository to make sure I don't lose my Hacktoberfest rewards in the worst scenario.

The project I contributed to this time was a really tiny one and all it did was throw a cool animation a cute anime character run across the screen and say her iconic dialogue Kuru-Kuru (not sure what that means).

I swear this was the funniest one I had come across so far. It was written in Preact that I had never heard of before, so was even more interesting to work on.

The issue was to implement an in-site ratelimiter, to prevent the Autoclicker scripts from abusing the button.

The author had some suggestions

But I went with the first one and opened a pull request.

This is what I did to implement a timer

  const THRESHOLD_CLICKS = 30; // Maximum number of clicks in an interval
  const INTERVAL_TIME_SECONDS = 60 * 0.5; // Every 30 seconds
  const [clicksInInterval, setClicksInInterval] = useState(0);
  const [intervalTime, setIntervalTime] = useState(0);

  const clickThresholdSurpassed = () => {
    return clicksInInterval >= THRESHOLD_CLICKS;
  }

  useEffect(() => {
    if (clickThresholdSurpassed()) {
      // Setup a timer
      const intervalId = setTimeout(() => {

        // Update interval time
        setIntervalTime(prevTime => prevTime + 1);

        // Reset interval if expired
        if (intervalTime >= INTERVAL_TIME_SECONDS) {
          setIntervalTime(0);
          setClicksInInterval(0);
        }
      }, 1000 * 1);

      return () => { clearInterval(intervalId) }
    }
  }, [clicksInInterval, intervalTime]);

...
...

{!clickThresholdSurpassed() && <Button id="ctr-btn" onClick={onClick}>Squish that kuru~</Button>}
{clickThresholdSurpassed() && <p class="text-red-600 font-bold">Too many squishes! Wait until {INTERVAL_TIME_SECONDS - intervalTime} seconds.</p>}

I learnt another important concept here as using setInterval (my initial approach) never updated the timer count due to the following reason.
https://www.reddit.com/r/reactjs/comments/occj3u/using_setinterval_with_usestate_together_wont/

For some reason, I decided to go with setTimeout approach instead as shown in the above code.

What matters is the maintainer was actually happy to have my contribution, a real positive interaction. I just made a couple more changes that were requested

and the pull request was finally merged.

I was officially the second contributor to Kuru-Kuru LMAO

Therefore, I would like to end this blog with some