fahminlb33 / Venjix

Multipurpose IOT data collection, visualization, and forecasting tool using ASP.NET Core MVC.
MIT License
3 stars 1 forks source link

Use of non local resources #12

Closed fahminlb33 closed 3 years ago

fahminlb33 commented 3 years ago

I just noticed that the new PR #5 is adding this line:

https://github.com/fahminlb33/Venjix/blob/2e78a33bf1c8347df1a3fbb17a89d8b7bc8b5417/Views/Shared/_Layout.cshtml#L21

It's not bad per se but it's better to use libman to download and use the library locally, so this app can be more portable in case of no internet connection.

fahminlb33 commented 3 years ago

@rasyidf Maybe you wanna take a look at this issue.

fahminlb33 commented 3 years ago

Another thing, is this icon pack necessary? Can we just use one? Either use FontAwesome or Feater Icons, don't use both as it will slow down the web and adds extra dependecies.

rasyidf commented 3 years ago

I guess feather icons is much smaller than Fontawesome, unfortunately, Bootstrap <= 4.5 are tightly coupled with FontAwesome, and making it harder to separate.

fahminlb33 commented 3 years ago

I've included Feather Icon in the commit 3c35add05176f78f117cb97676105fb252caf252 But why do we need separate icon pack? Can we just use FontAwesome instead?

rasyidf commented 3 years ago

Icons are designed to meet some use cases, while font-awesome is broadly used in small web iconography, and because of its legacy icons being converted to vector, it lacks of scalability. In terms of style, there are ionic, iconic, feathers, fluent and bootstrap Icons that uses svg and designed with scalability in mind, big icons and small one have good proportion.

Well, we can gradually remove font-awesome, for the sake of UI consistency. AFAIK, the bootstrap icons that tightly coupled were only at some chevron expander icon.

fahminlb33 commented 3 years ago

Okay, I understand your point of view, that's just from my point of view from optimization, but I lack UI/UX experience so I'm sorry for that. I'll close this issue now.