Mistakes to Avoid Before Submitting Your Pull Request
Tags: opensource, hacktoberfest, css
Published: 2021-10-27 03:40:26

After browsing through a lot of beginner-friendly issues, one [issue](https://github.com/vector-im/element-web/issues/19180) in the [Element repo](https://github.com/vector-im/element-web) 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_:

![Text overflow inside a button](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/f7x8d2o0n2ls7vtbtdst.png)

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

![Cat facepalm](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/u09rje5a2jha0jnuzzon.png)

Unlike any open source projects to which I've contributed, this project involves 2 other repos, [matrix-react-sdk](https://github.com/matrix-org/matrix-react-sdk) and [matrix-js-sdk](https://github.com/matrix-org/matrix-js-sdk). As explained in the [`Development` guide](https://github.com/vector-im/element-web#development) 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_:
```css
.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.
![Project member's comment](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/up4efjhwkm1zert5lm6f.jpg)

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](https://github.com/matrix-org/matrix-react-sdk/pull/6898#pullrequestreview-771069065) on how I could improve my code further by changing `height` to `min-height`.

Finally, [my PR](https://github.com/matrix-org/matrix-react-sdk/pull/6898) was completed and merged thanks to help from the members of the Element project.

_The same page after my fix was applied_:
![The text no longer overflows the button](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/h3n3ys8lv0bydnvrl0ph.png)

_How I fixed it_:
```css
.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](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v3.33.0). Nevertheless, at the end of the day, the two most important things that I've learned are the mistakes I mentioned above.