search

generoi / genero-design-system

https://gds.generogrowth.com/
MIT License
4 stars 0 forks source link

Menu cleanup #51

Closed oxyc closed 3 years ago

oxyc commented 3 years ago

I added a deprecations.scss file that we can use to manage deprecations in the future.

In this particular case it's still breaking though because there's no way to migrate the previous --menu-item-underline-display: block; use case.

oxyc commented 3 years ago

@taromorimoto do you think you have time to review this one during the week?

What do you think about the direction it's going? Any doubts? If you think everything is good I could merge this and the a11y and we can focus on the breaking changes. But since it's a big opinionated change you might not agree with everything :)

Maybe it's good to keep it in a v4.x branch too though? in case you need to merge other things for your current project and we dont bump the major version until we're done with breaking changes.

taromorimoto commented 3 years ago

@taromorimoto do you think you have time to review this one during the week?

What do you think about the direction it's going? Any doubts? If you think everything is good I could merge this and the a11y and we can focus on the breaking changes. But since it's a big opinionated change you might not agree with everything :)

Maybe it's good to keep it in a v4.x branch too though? in case you need to merge other things for your current project and we dont bump the major version until we're done with breaking changes.

Yeah, let's keep all these changes in v4 branch. I'll take a look at this today.

taromorimoto commented 3 years ago

I added a deprecations.scss file that we can use to manage deprecations in the future.

In this particular case it's still breaking though because there's no way to migrate the previous --menu-item-underline-display: block; use case.

I wouldn't be too worried about the deprecations.scss file. I think it should only be best effort.

taromorimoto commented 3 years ago

@oxyc I don't see any problems with the changes you've done. I think they are good and it's great that finally get accessibility.

I made some comments but other than that, lgtm.

oxyc commented 3 years ago

This has been incorporated in the original a11y PR.