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

Sass & HAML #50

Closed
meneerprins opened this issue Feb 7, 2018 · 16 comments
Closed

Sass & HAML #50

meneerprins opened this issue Feb 7, 2018 · 16 comments

Comments

@meneerprins
Copy link

Would it be possible to just set the template language in an initializer? I use the hamlit gem (not haml-rails or something) but now komponent generates .erb files.

And for sass/scss, could it be possible to import one index file with all sass component partials? Since variables can't be used globally when each file is imported by the javascript.

@florentferry
Copy link
Member

florentferry commented Feb 7, 2018

Hello @meneerprins ,

For the first point:

Rails.application.config.generators.template_engine = :haml

For the second point, I need to investigate and discuss with @Spone about the need and find a solution to make it possible without list all files into one globally.

@florentferry
Copy link
Member

florentferry commented Feb 7, 2018

A possible solution is to create a file _variables.scss in frontend folder and import it in each sass/scss files you need variables.

// frontend/_colors.scss

$color: yellow;
// frontend/components/button/button.scss

@import '../../_colors';

.button {
  background-color: $color;
}

@meneerprins
Copy link
Author

Hi @florentferry thanks for the tip, didn't know about that setting for HAML.

For the scss, that just doesn't look very practical, especially when you have a large project with a lot of components. In addition to that, variables are used in almost every file in my experience. Some sort of autogenerated _components.scss partial (sorted alphabetically) that you can manually include would be a nice addition imo.

@Spone
Copy link
Collaborator

Spone commented Feb 8, 2018

Hi @meneerprins, good idea! We'll do some tests and keep you informed.

@florentferry
Copy link
Member

Solution 1:

  • Create a new file application.scss in frontend/packs
@import '../colors';
@import '../components/index';
  • Require it in application.js in frontend/packs
import './application.scss';
import 'components';
  • Create a new file index.scss in frontend/components and require components stylesheets into it.
@import './button/buttton';
  • Remove import './button.scss'; from frontend/components/button/button.js

So now, each components stylesheets have access to variables globally defined in colors file.

@Joralf
Copy link

Joralf commented Mar 22, 2018

Hi @florentferry and @Spone,

I've ran into the same issue as @meneerprins. Your solution solves the usage of variables in components, but the price is that all imports done from the css file in the component break.

Say the button uses the following css:

.button {
    background-image: url("./button-background.png");
}

where the image is located at ./button/button-background.png.

With the proposed solution the loader will try to resolve ./button-background instead of ./button/button-background.png which shouldn't be the case as the image is part of the component.

@florentferry
Copy link
Member

Hello @Joralf,

I let @Spone respond to you, he uses Komponent in several projects, then he should have the same issue than you. I use the first method I mentioned, importing in each component.

@Spone
Copy link
Collaborator

Spone commented Mar 22, 2018

Hi @Joralf

I don't have this issue, where all imports break. Can you please create a gist with code to reproduce the issue?

@Joralf
Copy link

Joralf commented Mar 23, 2018

Thanks for the quick reaction guys!

@Spone, here's the gist: https://gist.github.com/Joralf/41923d3a22245c8dbd5e790b9e58aeee

This throws an error with:

ERROR in ./frontend/packs/entry.scss
Module build failed: ModuleNotFoundError: Module not found: Error: Can't resolve './test.jpg' in '/Users/joralf/Sites/myapp/frontend/packs'

Which makes sense, because the entry point of the scss file is now packs/entry.scss which means any url resolving is done from that path. You can "solve" this by changing the line to background-image: url('../components/test/test.jpg');, which means the css in the component needs to be aware of the path and is no longer "standalone". That would be a huge dealbreaker for me.

Another potential issue with the proposed solution is that all css is now accessible by any component in the tree. To prove this I've created a second gist:

https://gist.github.com/Joralf/17981dc7950ace3c207e1fe58413fb6a

When using the default Komponent way, css defined in the component folder is only accessible by the component itself (as it's loaded via the .js file in the component). With the proposed solution this is no longer the case.

@Spone
Copy link
Collaborator

Spone commented Mar 24, 2018

Thanks @Joralf for taking the time to prepare the gists.

I agree with you that the most important here is to have "plug-and-play" components with proper encapsulation and no dependency to their path.

I feel that using a single entry point for each component (the .js file) is more true to the Webpack way. But it comes at the expense of not having access to variables without explicitely importing them.

Maybe we could keep it the current way, but include the variables CSS file to the Komponent install generator, and then import it in the default CSS template used by the component generator. This way, you wouldn't have to add the import manually for each component.

What do you think?

@Joralf
Copy link

Joralf commented Apr 3, 2018

Hi @Spone and @florentferry ,

That sounds like a good solution. So it's a similar approach to #82 except that the css file you want to include on top will be dynamic?

@Spone
Copy link
Collaborator

Spone commented Apr 3, 2018

Not sure what you mean about #82

@Joralf
Copy link

Joralf commented Apr 3, 2018

Ah sorry. Maybe wrong comparison. But as far as I understand you want to make sure that:

@import '../../src/stylesheets/variables';

Will be added automatically to the top of every css file when you generate it via rails generate component xxx

@Spone
Copy link
Collaborator

Spone commented Apr 3, 2018

Yes exactly. But I think that means we have to enforce a convention for naming this file.

For instance I use ../../base/variables.scss but you use ../../src/stylesheets/variables

Also, I think we want to @import with a path based on Komponent's root, so we don't need the ../../ in front of the path. We would be able to simply do @import "base/variables";

Would that work for you?

@Joralf
Copy link

Joralf commented Apr 11, 2018

Yeah, I'm fine with forcing the name/location for the file. And that import statement would make it a lot easier!

@Spone
Copy link
Collaborator

Spone commented Apr 11, 2018

Ok, I created a issue for that: #88 and I'm closing this one.

@Spone Spone closed this as completed Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants