DEV Community

Yuan-Hsi Lee
Yuan-Hsi Lee

Posted on

Fix the previous mistake

In this post, I want to talk about my a PR I sent about 2 months ago, and how do I pick it up and improve it to meet the requirements.

Old issues

In the Hacktoberfest, I gave myself a new challenge, browser extensions. I checked many repos and worked on some issues. One of them is to add keyboard shortcuts for image size selection. In my last PR, my code works fine, but it's duplicate, long, and seems to be breakable.

There were some issues in my old PR, firstly, I use some brittle attribute names to get the elements which might be broken once the class names have been changed.

const openTool = document.querySelector(
        '[class="PNyWAd ZXJQ7c"][jsname="I4bIT"]');
Enter fullscreen mode Exit fullscreen mode

Secondly, I neglected the usage of different languages.

if (dropDownWithSize.innerHTML == 'Large') { /*...*/ }
Enter fullscreen mode Exit fullscreen mode

In other languages, the innerHTML values are not Large, therefore, this line of code will not work.

Thirdly, I didn't test all the scenarios. In some cases, the variables I defined might be null or undefined, however, my functions didn't check their value before using them, which will be causing errors while running the extension.

Lastly, there were several parts of my code that were similar or even duplicate which could be combined and reuse.

New solution

To solve these issues, I did some research in order to find alternatives. For example, instead of checking the innerHTML value, I go up to the parent component and check the aria-label value. In this case, the aria-label won't be affected in different languages.

if (dropDownWithSize.getAttribute('aria-label') == 'Large') { /*...*/ }
Enter fullscreen mode Exit fullscreen mode

To utilize and shorten my duplicate code, I use the basic method, drawing flowchart to find the solution to design my code. In my case, I need to reopen a dropdown to re-select an element. However, this element should be reused instead of using getElement function call again. I design a function with getElement, therefore, once I finish dealing with the reopened dropdown, I can simply call my customize getElement function again to get the same element, but with less code.


After truly working on solving this old issue, I realized that I overestimated the difficulty of it. When I receive the owner's change request, I was anxious and didn't have the confidence to make these changes. However, it turns out that I might actually able to solve these issues that I thought I couldn't, I just need to calm down, breaking things to pieces, and make a plan.

Top comments (0)