Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shadow alias for elevation #65

Closed

Conversation

shannonrothe
Copy link
Contributor

@shannonrothe shannonrothe commented Mar 1, 2022

@madeleineostoja I've gone with the naive approach for #64 here 馃檪

I thought it might be cool to provide some kind of "alias" map to formatModule, that way things can easily be aliased and you don't need to worry about duplication. For this naive solution, any change to "elevation" would require "shadow" to be updated too. I'm happy to explore that some more if you think that's a better way forward 馃憤馃徏

Approach 1: 451c93e (naive)
Approach 2: 40cdc51 (more flexible/scalable)

@madeleineostoja
Copy link
Contributor

Wow express! Thanks for this! I'm not even 100% sure on replacing elevation with shadow anymore after reading some more comments, but this is great. What's your take re: the names?

If we were to replace elevation with shadow then the basic approach makes more sense. If Pollen was to support true aliases then the other approach is a solid start. I feel like that's something that could be exposed via config as well, though I'm not sure on the utility of aliases, probably just muddies the waters for consumers of a design system (pollen's defaults or a custom one)

@shannonrothe
Copy link
Contributor Author

What's your take re: the names?

IMO shadow is more obvious than elevation but since that would be a breaking change I can see the dilemma 馃槄

@madeleineostoja
Copy link
Contributor

Yeah I think it is probably a good change. Someone mentioned that elevation makes a lot of semantic sense (ie: conveys layers rather than just misc shadow effects), but idk. I might just leave it open for now and see if anyone else wants to weigh in. At the end of the day can always merge and just strip one or the other out next major version, in the meantime it鈥檚 just whatever we put in the docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants