Help me choose
Internship/2021 year-end summer internship

[WK7-F] Updating snapshot, MUI decouple style from component

by hajinny 2022. 1. 28.

1. Waking up

Yesterday, I had the most intense cardio session of my life. It was 4km run, 10min rowing (2km), 2km run, 5min cycling (2.35km), which all amounted to 50 minutes. My whole tshirt got wet, and my leg was bascially cramped. 

 

Anyways, I managed to get enough sleep that I could wake up at 8:40. 

2. Updating snapshot late after merging PR

I woke up to a slack message that said the snapshot wasn't updated for my latest merged PR (causing snapshot failures), so I hurriedly went to my room and fixed that. The procedure was just branching out from master and making branch of the same name as the last PR (as the prev one I already deleted after merging). Then I made a PR from that and got approvals then merged it. Then went to jira to close the ticket.

3. Mui decoupling style from a component (feat add div and do child selector)

From my yesterday's post, you would have realized that I was coming up with a solution to style SkipToMain component while making sure that SkipToMain is decoupled from that styling information. 

 

This was my previous attempt:

// very simplified view of App component, top level component
import {appStyles} from '../appStyles'

function App({...}){
    const classes = makeStyles(appStyles)()
    return (<>
        <SkipToMain className={classes.dataSkipNavLink}>
        <main>...</main>
        </>
    <>)
}


// appStyles.js
export const appStyles = (theme)=>{
    dataSkipNavLink:{
    	'&:focus':{
        	zIndex: theme.zIndex.modal
        }
    }
    // other styles...
}


// SkipToMain.js
const SkipToMain = ({..., className})=>(
	<>
    	<a className={className}>...</a>
    <>
)

I raised a PR for that. However, it's clearly with a fault because SkipToMain now has to accept className as an extra prop. If className was some generic prop that we need every ui component to have, we wouldn't worry too much. However, className was added to add a styling that SkipLink wouldn't otherwise need if it weren't working with other components in our product's context. Dumb ui component shouldn't need to care about that. Adding className essentially reduced interoperatability, increased coupling and reduced reusability.

 

The solution is obvious if working with pure css (just do child selector), but you need to know that '& a' is a child selector in order to come up with the following solution.

const appStyles = (theme)=>{
    dataSkipNavLink:{
        '& a':{
            zIndex: theme.zIndex.modal + 2
        }
    }
}

function App({...}){
    const classes = makeStyles(appStyles)()
    return (<>
    	<div className={classes.dataSkipNavLink}>
	        <SkipToMain />
        </div>
        <main>...</main>
    </>)
}


// SkipToMain.js
const SkipToMain = ({...})=>(
	<>
    	<a>...</a>
    <>
)

This is much better. Now, SkipToMain knows nothing about how the parent component is styling it. I've added a div surrounding SkipToMain, that gets a styling of class name 'dataSkipNavLink'. The reason I did this is because we can't style a custom react component like SkipToMain directly through className or style, which get passed as props - took me a while to figure that out. I've used '& a' (descendant selector) to make sure that the style of zIndex applies to a tag of SkipToMain while I apply className to the div surrounding it. Note that <> (fragment) doesn't have any impact to DOM, so effectively we have <a> being placed as a direct children of <div className={classes.dataSkipNavLink} >.

 

So here, we have moved styling stuff (of zIndex, that SkipToMain shouldn't need to know) to the App component and made SkipToMain dumb.