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

Component styles #1295

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Component styles #1295

wants to merge 2 commits into from

Conversation

asolove
Copy link
Contributor

@asolove asolove commented Sep 11, 2013

What's done

  • Add a map of style axes to Component, configurable via properties or bindings.
  • Automatically add/remove CSS classes for the current value of each axis.
  • Style classes are prefixed by the new componentClassName attribute, e.g.: digit-Button--size-large

What's not done

  • Style properties are nested inside styles, so you can't declare them in a blueprint or set them from the Lumieres inspector. Can someone help me create a StylePropertyBlueprint that is a subclass of PropertyBlueprint buts sets values inside the styles map?
  • There is currently no support for specifying which axes and values are supported, their defaults, or introspecting on them at runtime. So you have to specify values for all style axes in the serialization. Options: leave it how it is, read the blueprint in at runtime, duplicate the declarations in the class file, or ...?

What it looks like

"button": {
  "properties": {
    "styles": {
      "size": "large"
    },
  },
  "bindings": {
    "styles.get('highlighted')": { "<-": "@form.valid" }
  }
}

@kriskowal
Copy link
Member

I would say yes with regard to componentClassName ?? constructor.name, though constructorName might be more generic.

Coercing styles assignment to the internal Map representation is cute. That’ll need some explaining and you could get the ball rolling in the JSDoc, and—if you’re energetic—ui/component.md, and—if you’re really energetic—in an article on components on montagejs.org.

In your example, do you mean highlighted and large to be the same name?

@Stuk
Copy link
Contributor

Stuk commented Sep 11, 2013

Is it ok to add a magic componentClassName property that component authors can fill in and that defaults to the string name of the constructor?

That makes sense. Montage.getInfoForObject will come in useful.

@asolove
Copy link
Contributor Author

asolove commented Sep 11, 2013

Another item: right now the style properties can't appear in blueprints (and be set in the Lumieres inspector) because they're nested inside the styles property. I think I am going to need some help understanding the blueprints and deserialization well enough to write a subtype of PropertyBlueprint for StylePropertyBlueprint that assigns into object.styles rather than object.

@kriskowal
Copy link
Member

I’m passing this to @Stuk to get it out of limbo.

@asolove
Copy link
Contributor Author

asolove commented May 28, 2014

Wow, is this the official "everyone work on Adam's old tickets" week? I feel strongly about this one and am happy to help if I can...

@kriskowal
Copy link
Member

@asolove, Not specifically. @mczepiel introduced me to Bee and I have been going to town on issues with no assignees.

@hthetiot hthetiot added this to the Future milestone Apr 12, 2017
@hthetiot hthetiot requested a review from marchant April 12, 2017 17:10
@hthetiot hthetiot removed the request for review from marchant May 1, 2017 20:09
@hthetiot hthetiot requested a review from marchant July 13, 2017 02:20
@hthetiot hthetiot modified the milestones: v17.2.x Component, Future Jul 13, 2017
@hthetiot hthetiot requested a review from cdebost July 13, 2017 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants