DEV Community

Alexander Samaniego
Alexander Samaniego

Posted on

DPS909 Blog: Final Pull Requests

This post is an update on my progress on the issues I selected to work on in my previous post for Release 0.4.

Pull Requests

I completed my work on issue #201 and PR #224 for that issue. The PR was eventually merged into the main branch after a couple of necessary adjustments. I ended up having to adjust the querySelector in order to remove a condition. The querySelector now only selects elements with the attribute data-validation-on. This allowed me to simplify the code while only selecting elements from the page that were created by the library. Furthermore, I had to add a nullish operator to set the default event listener if there is no attribute specifying one. I also added optional chaining to check whether the attribute actually exists, to avoid any errors.

As for the other issue I wanted to work on, #211. It was slightly more difficult than I had thought. I spent a couple of days experimenting on how to enable validation on forms when the user optionally uses <validation>with a form passed in as a prop, as explained by the second point in this comment. I could not find a solution for this because I realized in Astro, you can't use prop component variables within the <script> element in a component. Outside a <script> element, it is possible to use component variables as described in the documentation. So, this is where I had trouble trying to apply event listeners to forms specified by the user after form creation. I didn't get a solid enough solution to create a PR for this issue. I asked the project maintainer a couple of questions for clarification, but they ended up closing the issue due to a change of approach.

So with things not going as planned, I quickly had to find another issue related to the form component. Luckily there was an issue directly related to the first PR I made. Issue #230 relates to the form validator again. All components except the Radio buttons were compatible with form validation. To fix this issue, I spent time experimenting with various ways to make the Radio buttons compatible. At first, I was looking at the validator file, Validator.astro, for any line of code that may have caused the error. When nothing came of it, I went through the process of form creation multiple times to see exactly why other components were being validated and not the Radio buttons. I realized the this library treats radio buttons as one form component when in reality it is actually multiple. So, I decided to look at the other components to see what is different about the Radio buttons. As I described in my comment here, I noticed that other components only contain <input> and <label> elements, whereas the Radio button element contained another <div> element that covers the other elements. I experimented with removing that element and found that validation works. I suspect that the additional element was interfering with validation of that component and created PR #234 describing my solution. It still needs to be reviewed and merged but I think that part will go smoothly.

Conclusion

I think this entire process was a good experience. I got to work on a project that I was already familiar with and further enhance my knowledge of it. I also got to work with the project owner and maintainers while working on my issues. Without their help the process would've taken much longer and it gave me more experience working with "strangers" on a public repository. I felt like I gained their trust with every successful PR. So while things may not have gone as planned, I still felt good about closing several issues while making 2 PRs. Things not going as planned also showed me that not everything goes smoothly so you have to think quickly to get things done. My goal was to complete at least 2 PRs and I got it done, even though it wasn't one of the issues I originally wanted to do.

This should be my last blog post for DPS909. This has been one of the most useful courses I've taken to date and I've come out a better developer than I was at the beginning. 10/10, would recommend.

Top comments (0)