Blazored / Gitter

A Blazor Gitter Client
MIT License
37 stars 14 forks source link

2019-04-30 WiP on scss consolidation #37

Closed MarkStega closed 5 years ago

MarkStega commented 5 years ago

This is a WiP; SCSS has been moved to the Core project; The following issues exist: (1) The server project does not seem to be able to access LocalStorage (2) The server project successfully gets a login as long as you do 'login once' to avoid the localstorage issue but then never seems to populate the rooms. (3) The client project in Chrome receives a CORS error for the font and gets an access denied error, it works in Firefox & Edge

chrissainty commented 5 years ago

Thanks for this @MarkStega, I've had a scan over and I've got a couple of small things I spotted.

I see you've put the scss files into the content folder, could these be moved outside of this folder, please. The content folder is for static files, JS, CSS, Images;scss files are source files. The outputted CSS file should be in the content folder.

Can you rename the outputted .css file to be blazored.gitter.min.cs rather than blazored.gitter.core.min.css

There seems to be a loader.css being compiled in the client project and the server project. I'm curious as to why this a separate file? If possible I would rather produce a single CSS file for the app and I can't see any reason why this single file is split out from the rest.

MarkStega commented 5 years ago

@chrissainty @SQL-MisterMagoo

I'll follow your lead since this is your project but let me explain what my reasoning was in the distribution of the styling:

At the point that either the client or the server project is rendering the index.html or _host.cshtml file the resources that are embedded in the core project are not yet available as the blazor app (so to speak) is not running and the blazor embedd library is not yet invoked to get the static resources. This is the reason for the separation of the loader styling from the core styling. I suppose this might change for P6 but it doesn't seem likely. I could, of course, move the source scss file to the core project if desired but I believe I still have to have the css available in the 'root' of either the client or server projects. One other way to accomplish this is of course to have the source files in a separate project, not use the embedded resource, and have all of the css included in either the client or server project. I am not as fond of this solution because it seems that the core project should be as close to stand-alone as possible. But that is a general sense, I guess it unlikely that the core could be used separately.

The reason for the blazor.gitter.core.min..css file name is that if I have separate 'components' (not to be confused with Razor components) like that built in the core project. I like having the resources for that set of files composing the project named essentially the same as the project itself. Personal preference so I'll name the css input & output whatever you want.

And finally, I've don't think I've ever seen guidance on what the content directory should or should not include. Obviously the csproj drives what gets embedded, I just like having the source & output close, but again, personal preference, so I'll move the source to another directory if you feel strongly.

Anyhow, I hacked against the server side this afternoon with no real progress on understanding the flow issue.

Just let me know your collective thoughts & I'll modify as requested.

chrissainty commented 5 years ago

@MarkStega @SQL-MisterMagoo

Thanks for the clarification Mark. I see your point on the loading styles and that makes sense now.

You're correct on the guidance on the content directory. This is me applying rules from work, apologies, I should have stated that. Let's leave things where they are for now. I would still like to drop the .core from the main .css file name though.

SQL-MisterMagoo commented 5 years ago

I'm going to change all the "event Action" code to use EventHandlers instead - it seems the Actions are not firing in server mode - obviously I'll make sure both modes work after I make the changes...

MarkStega commented 5 years ago

This is building & working correctly on my system in both client & server mode. The detail of the build failure in the CI is rather sparse (cmd.exe exited with code 1). If you can give me a hint I can try to figure out that issue. The problem with the LocalStorage was resolved by the move of the embed library to app.razor/cs in PR#38.

SQL-MisterMagoo commented 5 years ago

@chrissainty I found this issue https://github.com/madskristensen/BundlerMinifier/issues/365 Jon Skeet suggests setting an environment variable MSBUILDDISABLENODEREUSE=1 Could you try that to see if it lets this branch build?

chrissainty commented 5 years ago

I've updated the build script in master, @MarkStega can you merge it into your branch

SQL-MisterMagoo commented 5 years ago

@MarkStega @chrissainty I've pulled this branch locally to test and it all looks good to close #33

I'll leave it to Chris to have the final say as he knows SCSS and I don't.

MarkStega commented 5 years ago

@chrissainty @SQL-MisterMagoo I pushed the yml pipline change, not sure why it shows as a conflict. It's probably my love/hate relationship with git :-)

SQL-MisterMagoo commented 5 years ago

Thanks @MarkStega for this contribution, it will be a great help now we have this centralised and can use the server mode for debugging !