[WK9-W] Open source contribution
1. Waking up
I woke up right at 9am today. I played maplestory on mobile till 1:30am so that's what made me super tired. Even while I was playing I was keep thinking, "maple is really boring" until I realized that I spent like an hour playing it. It's not bad XD
2. Open source contribution, let's raise a PR!
2.1 The fix
This is obviously a follow up to this post from yesterday. Yesterday, I understood the whole process of making changes and seeing the change reflecting on the demo application, by linking local repository as a dependency to our project. That process was quite interesting. By the way, since our project supports hot reloading, it seems I don't have to rerun our project application everytime I make changes to the repo (I just need to build it from the side of repo). Anyways, today I found the fix that solves the issue that was outlined yesterday.
The fix was quite simple actually. For simplicity, let's say that Carousel component looks like this
const Carousel = ()=>(
return (
<div
onMouseOver={()=>this.stop()}
onMouseOut={()=>this.reset()}
>
{children}
</div>
)
)The good thing here was that there were functions to stop and start the autoplay on mouse hover and mouse exit. The only thing I had to do was to hook onto an element and assign those functions to onfocusin and onfocusout properties. Where did I get these 'onFocusIn and onFocusOut' stuff from? Well, there was an example from w3 that showcased a prototype carousel that meets all the WCAG accessibility guidelines for carousel, and the code too:
However, the difficult part was to find exactly what element to hook those functions to, and for what properties of that element those handlers should be assigned. The thing that doesn't help is that we are using React, meaning those focusin, focusout events aren't really one to one in terms of naming. So I experimented a bit to begin with, dabbling around stackoverflow.
const Carousel = ()=>(
return (
<div
onMouseOver={()=>this.stop()}
onMouseOut={()=>this.reset()}
onFocus={()=>this.stop()}
onBlur={()=>this.reset()}
>
{children}
</div>
)
)Heck, that's too simple right? But there was a lot of digging, testing, rebuilding that went into this. In particular, I had to keep rebuilding the library every time that I made changes, and I was always uncertain whether what I do will do what I want it to do (for one, onFocusIn / onFocusOut didn't exist for div in React, and it was purely coincidence that my eye landed on 'onBlur').
Testing was done through Storybook. Storybook had a carousel component story that autoplayed, so it was easy to confirm that my change actually enabled that keyboard accessibility feature (autoplay stops if keyboard focus is on any children of carousel). Our application doesn't currently have an autoplaying carousel, so I would've needed to temporarily change the application code to test if we didn't have Storybook. It just felt nice not having to change any code on the application.
2.2 Raising PR
Okay, it's someone else's repository whose write access is blocked. So how do I make a PR? Here's the step:
1. Fork repo so you get a repo under your namespace
2. Make a change
3. Create a pull request
Very simple! Good thing is that Github desktop picked up the fact that I couldn't commit to Learus' repository, and asked me if I wanted to fork. I pressed okay, and it just created a fork like this:
Then I commited my changes to a new branch, and raised a pull request from Github. The process was really seemless, and I didn't know it was this easy! You can raise PR from your repository and it will know you want to raise PR to the original repository that you forked from. Here, I raised PR from a branch called enhancement/pause... of my forked carousel repo to branch `version2` of Learus' carousel repo.
That's it!