Consistent styling for Error Page (404 page)

[Vall3y]

I have applied the same style from <InANutShell /> to the error page. To do so I have created a shared styles folder under web_modules/app, similarly to how it is done in this CSS Modules demo (See StyleVariantA.css)

I didn't want to refactor any more code before getting some feedback if this style is acceptable, let me know what you think

[marcustisater]

Thanks for the PR. I will take a look.

[jeddy3]

@Vall3y Thanks for getting stuck in :)

I'm easy if you guys want to go down the route of abstracting styles out of components, but in my experience it makes maintenance more difficult by increasing odds of side-effects and the cognitive load on developers. I intentionally avoided these kind of abstractions when I first built the homepage. I added, at the time, a small note the CSS Processors section of the react guide to help explain why:

"It is possible to compose selectors with CSS Modules. However, do not compose class names from other CSS Modules as dependencies make it more difficult to managed side effects in CSS."

That's not to say you can't use composition inside of a component, for example inside of the Social component. I think there's less for a developer to mentally model in these instances and the side-effects are limited to within the component.

Anyhow, like I said, you guys can take this down which ever route is most comfortable for you. I just figured you'd appreciate knowing the original rationale behind not using these type of abstractions :)

[marcustisater]

I agree with @jeddy3, that's a very good point indeed. I'm speaking as a novice CSS Modules user and in my opinion composing selectors outside components is very much like a bad practiced "extend" creating more unnecessary complications rather then good use.

Irrelevant example but after being a very long Sass time user I know such things are tempting to reduce writing but sorry to say I have always disliked it because usually it creates more side effects.

I'd say refactor this, let's not abstract styles out of components.

[marcustisater]

Irrelevant(sorry): @jeddy3 - did you get my issue message? You should jump into gitter! :+1:

[marcustisater]

@Vall3y - Would you like to refactor this from the feedback or close for new PR?

[Vall3y]

Thanks for the clarification, I do appreciate that. I can refactor this PR

So I understand that the preferred method is for the React class to import and apply the common styles itself, rather than for the css classes to be composed. Is this correct?

Another solution that makes sense to me is turn the basic typography units into components. The other solution would be to have the each component have a duplicate of the style statements which I think is not a good idea. Let me know what you think though because I am not sure how to proceed

In the post @jeddy has linked, Phillip Walton actually seems okay with css classes extending others, even though I do understand why your solution might be better

[thangngoc89]

IMHO, composing is a bad idea. I usually refactor React component to avoid that

[jeddy3]

So I understand that the preferred method is for the React class to import and apply the common styles itself, rather than for the css classes to be composed. Is this correct?

I think the thing to do here is not spend time abstracting out common styles, but rather spend that time designing and building the styles for an unique looking error page. Something that clearly distinguishes it from a "normal" page, so that it better communicates to the user that something has gone wrong.

The other solution would be to have the each component have a duplicate of the style statements

We don't have to worry about this problem then... and we end up with a nice looking error page :)

[Vall3y]

I have reverted the abstraction changes, and added some of its own style. I've also added a redirect to homepage message. Let me know if this is okay

(I amended the changes so the previous commit is gone)

[jeddy3]

LGTM :)

[marcustisater]

LGTM

Thanks everyone for the discussion of abstracting styles, got me thinking as well.