[docs] Generate unique hash

[mbrookes]

• I have followed (at least) the PR section of the contributing guide.

Sorry for yet another hash / TOC related PR (no more, I promise! 😄 ) – but when I was working on the other two, I couldn't help notice the number of duplicate id warnings we were generating.

Warning is nice, but not creating duplicates in the first place is better. This PR fixes that.

(Also DRY up the TOC JSX slightly.)

Here's a good page for comparison:

• Before
• After

Check the "Styling solution" links. There are three duplicates, and all three will be highlighted in the TOC when any of them is active.

BTW, I sat on this for PR for a few days, as I couldn't figure out why the active TOC entry wasn't updating correctly, or how this PR could have caused the problem. But since I "knew" that it was working perfectly in the last PR, it must be this one. It was only when I noticed the same behavior in the current published docs (#14628) that I realized it wasn't this PR after all, but rather a certain late modification to said previous PR.

All of which is a long way of saying, please merge this PR before that one. 😄

[oliviertassinari]

@mbrookes I'm refactoring the code :)

[mui-pr-bot]

Details of bundle changes.

Comparing: 6b68487...3fa7dda

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	0.00%	0.00%	368,621	368,621	91,396	91,396
@material-ui/core/Paper	0.00%	0.00%	76,647	76,647	19,297	19,297
@material-ui/core/Paper.esm	0.00%	0.00%	71,595	71,595	18,772	18,772
@material-ui/core/Popper	0.00%	0.00%	30,462	30,462	10,583	10,583
@material-ui/core/styles/createMuiTheme	0.00%	0.00%	17,286	17,286	5,720	5,720
@material-ui/core/useMediaQuery	0.00%	0.00%	2,486	2,486	1,054	1,054
@material-ui/lab	0.00%	0.00%	182,889	182,889	50,246	50,246
@material-ui/styles	0.00%	0.00%	57,708	57,708	16,237	16,237
@material-ui/system	0.00%	0.00%	17,062	17,062	4,486	4,486
Button	0.00%	0.00%	99,068	99,068	26,483	26,483
Modal	0.00%	0.00%	98,649	98,649	26,151	26,151
colorManipulator	0.00%	0.00%	3,232	3,232	1,297	1,297
docs.landing	0.00%	0.00%	49,899	49,899	10,728	10,728
docs.main	-0.06%	-0.05%	677,788	677,358	205,910	205,800
packages/material-ui/build/umd/material-ui.production.min.js	0.00%	0.00%	319,832	319,832	84,603	84,603

Generated by 🚫 dangerJS against 3fa7dda

[oliviertassinari]

@mbrookes Could you review the changes? I have tried something aggressive. I'm introducing the server side support back of the TOCs. I'm removing the click handler logic, I'm trying to rely on the browser default behaviors.

[mbrookes]

This is no longer working as intended.

Click on API in the TOC here: https://deploy-preview-14632--material-ui.netlify.com/style/icons

• Wrong TOC entry highlighted
• Wrong fragment id selected (although I"m having my doubts about that functionality)

For comparison:

https://next.material-ui.com/style/icons

[oliviertassinari]

It looks like an acceptable tradeoff for me. We win: a simpler logic, a fix of the active state after the first click (can be reproduced in https://next--material-ui.netlify.com/style/icons/).

[oliviertassinari]

We might be able to solve the "bottom of the page active link" problem by changing the findActiveIndex() method's logic.

[mbrookes]

simpler logic

...by removing functionality.

a fix of the active state after the first click

It works for me (or you need to be more specific about what's broken).

We might be able to solve the "bottom of the page active link" problem by changing the findActiveIndex() method's logic.

I can't think how... and I'd rather not spend more time on it. Let's leave this PR as it was.

[oliviertassinari]

...by removing functionality.

That's the tradeoff, kill features that are too hard to get right. Would you rather have the bottom top active issue or this one (the same item is active forever)?

this is the reproduction example in https://next--material-ui.netlify.com/style/icons/

[mbrookes]

Would you rather have the bottom top active issue or this one (the same item is active forever)?

Neither. https://deploy-preview-14548--material-ui.netlify.com/style/icons/#font-icons

[oliviertassinari]

Neither. https://deploy-preview-14548--material-ui.netlify.com/style/icons/#font-icons

I'm sorry about it, it's the problem when we solve hard problems, it's easy to break: #14629.
I could reproduce the same problem on https://developers.google.com/web/tools/workbox/guides/get-started#what_else_can_workbox_do, a website with 48M sessions/month according to SimilarWeb, I think that it's safe to ignore the problem. The fewer code we can get away with, the better. I have changed the active and focus pseudo-class style, inspired by what Google is doing.

[mbrookes]

I could reproduce the same problem on https://developers.google.com

Just because another site is broken, doesn't justify intentionally breaking this one.

it's easy to break: #14629.

Then we fix it whatever that broke, and move on. This code won't have to be visited again for a long time. And if it is, whoever changes it either needs to either test it more carefully (no offence @eps1lon), or write tests.

And in case you forgot 😈 :

Wow, I'm shocked by the simplicity of the code

#14548 (review)

have changed the active and focus pseudo-class style, inspired by what Google is doing.

I'm not sure what you were inspired by, but it looks broken:

[eps1lon]

Alright let's start fresh @mbrookes Can you describe the behavior you expect?

From what I heard it's expected that class based behavior does not translate well into hooks. This is why the react team recommends that only new behavior is implemented with hooks. The more complicated class behavior the more likely you end up with problematic hooks code.

Maybe it's better to throw the old code away and rewrite it completely instead of converting it.

[oliviertassinari]

Wow, I'm shocked by the simplicity of the code

It's even simpler now :). I think that it's a common schema in product development, supporting more cases does not always worth the extra complexity. It can be worse, drain energy away of what's important.

I'm not sure what you were inspired by, but it looks broken:

You might want to have a look at the menu item behavior then, same tradeoff for the hover and focus state. I doubt people use both.

[oliviertassinari]

@mbrookes I have noticed an important issue with the hash update logic, it's breaking the Next.js previous and next history. As we are talking about simplifications, what do you think of removing this feature too (so we don't have to invest time in fixing this problem)?