Mistakes to Avoid Before Submitting Your Pull Request

After browsing through a lot of beginner-friendly issues, one issue in the Element repo caught my attention. In brief, some text in languages other than English overflows a button in the Home page. I notice I've made several mistakes while completing the PR. If you are a first time contributor like me, you may want to avoid:

1. Skipping the contribution guidelines
2. Pushing outdated code

First look

Here's a picture of the issue:

It seems that a small CSS fix (revealed below) should resolve the issue, so I asked the maintainers of the repo to work on it. Not until I cloned the repo did I realize the tricky part was building the project for development.

I thought it was just another React project, so I made a pro-gamer's move: skip any type of tutorial. As I result, I wasted 2 days poking around the wrong repo.

Why you should not skip contribution guide

Unlike any open source projects to which I've contributed, this project involves 2 other repos, matrix-react-sdk and matrix-js-sdk. As explained in the Development guide in the element-web repo, I need those 2 SDKs in order to build Element successfully for code contribution.

The idea of Element is to be a relatively lightweight "skin" of customisations on top of the underlying matrix-react-sdk. matrix-react-sdk provides both the higher and lower level React components useful for building Matrix communication apps using React.

In other words, despite the issue being filed at the element-web repo, the code I needed to change was actually underneath the matrix-react-sdk repo. Such a plot twist, wasn't it? Lesson learned: reading the guidelines actually saves you more time.

After pulling the code from the right repo, the part of the code responsible for the bug could be easily identified. First, the CSS class of the buggy component can be looked up with Chrome DevTools. Then, a global search on Visual Studio Code (Shift + Cmd + F on Mac, Shift + Ctrl + F on Windows) can help you reveal its location if it exists in the codebase.

It was something like this:

.mx_HomePage_default_buttons {
        margin: 60px auto 0;
        width: fit-content;

        .mx_AccessibleButton {
            padding: 73px 8px 15px; // top: 20px top padding + 40px icon + 13px margin

            width: 160px;
            height: 132px;
            ...
        }
    }

Why you should update your branch

Once I had the fix, I immediately pushed my code up to my GitHub fork and made a PR. This resulted in my PR failing to pass the CI tests. I didn't change a lot of code, so what could have gone wrong?

One collaborator of the project pointed out to me something really useful.

Well, there were actually two useful things, but only one of them helped me pass the CI. There was another code reviewer who left a comment on how I could improve my code further by changing height to min-height.

Finally, my PR was completed and merged thanks to help from the members of the Element project.

The same page after my fix was applied:

How I fixed it:

.mx_HomePage_default_buttons {
        display: flex;
        ...

        .mx_AccessibleButton {
            ...
            min-height: 132px;
            ...
        }
    }

It was my first accepted PR in such a large project with more than 500 contributors. Although it was just a small fix, I couldn't help feeling proud while looking at the result of my contribution and my name in the project's change log. Nevertheless, at the end of the day, the two most important things that I've learned are the mistakes I mentioned above.