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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

figure out good paradigm for handling for mr-model's color #571

Open
hanbollar opened this issue Apr 9, 2024 · 2 comments
Open

figure out good paradigm for handling for mr-model's color #571

hanbollar opened this issue Apr 9, 2024 · 2 comments
Assignees

Comments

@hanbollar
Copy link
Contributor

          > ## Notes
  • there's a discrepancy in MaterialStyleSystem for background color and css understanding of what a model's color is since a model will also have a background - this will need to be figured out in a new issue/pr since it's more 'what would a user try to do in these different cases' research type question first
  • background color is being used for MRDivEntity's background (ie mr-model's background plane)
  • color is already being used for text coloring on that div
  • what to use for the model's actual coloring?

Originally posted by @hanbollar in #570 (comment)

@hanbollar
Copy link
Contributor Author

@lobau proposed this on discord:

mr-model {
  color: #000 /* the text color */
  background-color: #fff /* the backgroud color */
  material-color: #f90 /* the three js material color? */
}

@hanbollar hanbollar self-assigned this Apr 9, 2024
@hanbollar hanbollar changed the title css - what should we use for mr-model's color add css for mr-model's color Apr 9, 2024
@hanbollar hanbollar changed the title add css for mr-model's color figure out good paradigm for handling for mr-model's color Apr 9, 2024
@hanbollar
Copy link
Contributor Author

noting other thoughts from discussion this morning:

@michaelthatsit also proposed two alternative options:

  • sticking more with data-attributes for this since it's an off-case and we might not want to introduce a new css-specific variable since we dont want to add to many 'new' css features longterm, just add existing
    • pro: not adding a new css feature
    • con: not leaning into the use of the style-sheet for a style feature
  • overloading the color css attribute even if that's specifically for text (so if mr-text is a subtag, it would use that same coloring)
    • pro: handles the edge case of not introducing a new css property, but still getting one
    • con: really does not follow css paradigm as color is usually 100% for text only

and other things we talked about are pro/cons of this overall based on css proposal in above comments and prev ones as well

following this - going to sit on this and revisit in next version to see which makes most sense long term from a usabilty && maintainability perspective before we implement

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

No branches or pull requests

1 participant