idyll-lang / idyll-component

[DEPRECATED] Extensible component to be used in idyll projects
https://github.com/idyll-lang/idyll
MIT License
0 stars 0 forks source link

General approach for styling components #1

Open rreusser opened 7 years ago

rreusser commented 7 years ago

To prevent things from getting too inconsistent, it might be nice to suggest a method of styling components. Methods I can think of:

  1. Inline styles set by react. Downside: have to customize on every use or possibly wrap yourself just to style. How to do include media queries without coyp/paste by the user not clear.
  2. register each component's css location in its package.json. There's an idyll build step that somehow scans modules for this and collects all CSS in your build directory so it can be customized.
  3. require('insert-css')("...") in the component js so that it's added as a stylesheet once. Downside: still not customizable since it's effectively a global call from the component source.
  4. A configurable class method (e.g. Figure.createStyles) that's somehow used by idyll to create the CSS source? This seems unnecessarily fancy and awkward to get to feel simple and natural.
  5. A fancy react-ish css solution I'm not aware of?
  6. copy/paste by hand. I mean it works.
  7. ???
mathisonian commented 7 years ago

Okay, just to explicate priorities in considering this - it seems like the thing thats most desirable for you is access to the existing CSS so that you can modify it directly rather than relying on CSS overrides?

One radical option that I've been considering is for the "built in" components to be placed inside of a local folder during the project scaffolding step, rather than keeping them inside of the idyll module. This would enable users to locally modify the provided components and their styles.

mathisonian commented 7 years ago

The more that I think about that, the more I like the strategy of exposing those components (and their styles to the user) should they want to edit them. I think the main downsides are that it will make the generator a little more complicated, and it might require some restructuring of the idyll repo.

Curious for your thoughts @rreusser

mathisonian commented 7 years ago

I updated https://github.com/idyll-lang/idyll/pull/18 to use this strategy, so the generated project would now look like this:


mathisonian in ~/projects/idyll/idyll/test/test-project on build*
πŸ‘‰   tree -I node_modules
.
β”œβ”€β”€ _index.html
β”œβ”€β”€ components
β”‚Β Β  β”œβ”€β”€ custom-component.js
β”‚Β Β  └── default
β”‚Β Β      β”œβ”€β”€ styles
β”‚Β Β      β”‚Β Β  └── ... default component stylesheets
β”‚Β Β      └── ... default components in here
β”œβ”€β”€ data
β”‚Β Β  └── example-data.json
β”œβ”€β”€ index.idl
β”œβ”€β”€ package.json
└── styles.css
rreusser commented 7 years ago

Sorry I've been kinda out to lunch on feedback the last few days. Also apologies if I've skewed the priorities of your project in any bad directions with my feedback. (So yeah, the usual caveats πŸ™„)

Long story short, when I started getting fancy, I found myself undoing media queries one by one, which felt a bit fragile, especially if tweaking/augmenting the breakpoints at all. Copying everything seems like perhaps the best option (except for maybe the create-react-app approach of resolving things internally unless you explicitly ask it to copy things?). I think it's an improvement though!

I was contemplating the build/resolution process in my head the last couple days… A hypothetical way it could work (probably too fancy…):

  1. you run: idyll index.idl --build
  2. parses + constructs AST
  3. it gets a list of active component names ('link', 'regl', 'meta', etc.)
  4. scans components/, then node_modules, then node_modules up the tree, then builtin components. Idyll components could have, say an idyllManifest field in package.json to declare the what they actually expose. Scanning is a little expensive, but maybe not prohibitively so. Could filter obvious packages or just run once on startup to reduce the cost.
  5. now it has the AST and a component name --> source location mapping (that maybe includes other meta like stylesheet location too). From there, can build requires like it does now. The manifest could point to a stylesheet that would get concatenated into the current styles. And if you copied the source into local components/, that would take precedence in the resolution algorithm so that you could modify the styles directly there instead.

The main red flags are the fancy resolution step and the need for a sort of manifest. The upshot is that if you ran npm install idyll-fancy-component, it would be exposed automagically without having to wrap it yourself.

This all feels a bit too fancy, but it's the best I've been able to do in imagining how it might achieve that frictionless feeling where it's mostly unsurprising and doesn't really prevent you from adapting it to suit your needs.

As usual though, I'm not 100% convinced it's the right direction. Just inlining everything directly might be the best approach. Haha I'll stop here………

mathisonian commented 7 years ago

Alright, a few thoughts -

Here's the current algorithm, it's pretty similar to what you've outlined above, but I'll highlight differences below:

  1. you run: idyll index.idl --build
  2. parses + constructs AST
  3. it gets a list of active component names ('link', 'regl', 'meta', etc.)
  4. scans DEFAULT_COMPONENTS_PATH, then CUSTOM_COMPONENTS_PATH, with the custom components taking precedent in case of a name collisions. [note, this is equivalent to saying it scans each path in a COMPONENT_PATHS array with the later ones taking precedent, but I see this difference as an implementation detail].
  5. now it has the AST and a component name --> source location mapping, from there, it builds requires.

What this is missing compared to your suggestion is twofold:

  1. There is no concept of an manifest per component, currently a component is simply a javascript file (and whatever that file imports).
  2. There is no way to npm install idyll-fancy-component.

Now, addressing these deficiencies:

  1. There is no concept of an manifest per component, currently a component is simply a javascript file (and whatever that file imports).

For simplicity I'd like to try to avoid going in the direction of having a component manifest unless there is a really strong need for it. I think the components should be as lightweight as possible.

  1. There is no way to npm install idyll-fancy-component.

I'd really like to support this.

This seems like a feature that can be added in the future without interfering much with anything else. There would just be an additional scan step like you described above, where it looks through node_modules, and extracts anything with a tag of idyll-component in its package.json, or something like that.

The main issue would be that with this setup there is no way to directly manipulate the css from npm install'ed components. In this case, it seems like as long as component authors follow the guidelines of using insert-css rather than inlining all of their styles, users could still write custom CSS for these components relatively easily.

Copying everything seems like perhaps the best option (except for maybe the create-react-app approach of resolving things internally unless you explicitly ask it to copy things?). I think it's an improvement though!

This is how I'm feeling right now. We could always add that functionality to the generator, e.g. Do you want to copy the default components to your project? [Y/n]. If you choose no, it just updates DEFAULT_COMPONENTS_PATH to an internal path. Having this option doesn't feel essential to me though. I think the difference between this and CRA is that having the components copied into your project directory by default can serve as reference for which components are available - that is, having the files there is useful, they aren't just noise.