Skip to content

Optimizations / Changes? #2

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “No 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? No Sign in to your account

Closed
BeyondMagic opened this issue Jan 25, 2021 · 14 comments
Closed

Optimizations / Changes? #2

BeyondMagic opened this issue Jan 25, 2021 · 14 comments

Comments

@BeyondMagic
Copy link
Collaborator

A few things I am thinking after looking at the code.

  1. I think it would be easier for developers and people who made themes to put them here with PUG to render the HTML5 code;
  2. And to help optimizing and creating a better code, it would definitely be easier if it was made with npm, hence making easier scripts and live preview feature.
  3. Animations, they are heavy and take some time to load, maybe removing them or changing their load times.

I'd be happy to help with everything, I am just going to include a Pull request with some grammar corrections along this post, too.

@unseen-ninja
Copy link
Collaborator

While I personally really like PUG I don't think that it would make the process of adding a new theme easier per se. I'm currently trying to figure out a comfortable way to make the magic happen. My idea is to have some sort of simple database for now (like a json file or maybe even a new line seperated list) and have the HTML be generated from that.

For now I think it's fine to stick with the HTML5UP Template and maybe take some time to develop something that might be better suited for this. There is no need to rush it and eventually have more revisions than might be needed.
The loading times can be an issue tho, that's true.

@BeyondMagic
Copy link
Collaborator Author

@andreasgrafen One of the reasons I included PUG is that it has the option to load JSON files directly from a npm cli script, but I see where you're coming from and with just a separated file (like JSON) it wouldn't make difference whether the HTML5 render is.

The loading times can be an issue tho, that's true.

I don't know if many would think the same, but CSS animations with big elements are too laggy for me. One thing that may be causing other issue is that at loading time of the page my internet download goes up to 45MBs, can anyone confirm? I think that's a lot for any type of page.

@unseen-ninja
Copy link
Collaborator

Yeah, pug can do a lot of things.But as with every preprocessor it needs to be compiled down to HTML. I don't know much about GitHub Actions at this point.. I usually compile my code on Commit with GitLab's CI/CD feature on my own instance but I don't know if something like this is possible with GH. For this reason I've tried to come up with a solution that can be done user-side – at least for now.

I don't have issues with CSS Animations at all, but I'd still avoid it for larger elements because it can look weird quite quickly.
Loading all the images might cause performance issues as well. Serving the images in different sizes depending on the useragent specification could prevent this issue.

@Neikon
Copy link
Contributor

Neikon commented Jan 25, 2021

Both proposals are beyond my limited knowledge, so I can't contribute to the conversation. If you want you can try anything in another branch or fork and see how it works. and if it works we implement it. If you need me to invite you as a member or to add something specific, just let me know.

The website is only 6 megabytes right now as far as I can see in the firefox network console. So the problem of it weighing 40mb has been corrected.

@BeyondMagic
Copy link
Collaborator Author

@andreasgrafen Indeed, loading the full image right at the start is not good in long-term. as @Neikon said, I am going to try a few things and if they are nice, I send a pull request here.

@unseen-ninja
Copy link
Collaborator

I've just added a somewhat quick and dirty proof of concept in a seperate branch. Neither it's perfect nor fully functional, but I've pushed this for you guys to look at what I had in mind.

@Neikon
Copy link
Contributor

Neikon commented Jan 25, 2021

I've just added a somewhat quick and dirty proof of concept in a seperate branch. Neither it's perfect nor fully functional, but I've pushed this for you guys to look at what I had in mind.

I download to my desktop. The web load fine and see the footer but it don't show any theme (maybe i did something wrong). But i saw your idea and code. I like it. it is very simple and easy to add future themes

@unseen-ninja
Copy link
Collaborator

Oh, yeah. That could be a CORS issue since I'm loading a the JSON file from the local source and Firefox doesn't like that by default. Ofc it wouldn't behave like this in the real world. A temporary fix would be to set privacy.file_unique_origin to true in your about:config..
The order in which the JS is executed doesn't add up as well. The even listener for the on-click to open up the bigger view doesn't work with the items that are added via JS.

You're right, that's def. not a super fancy solution; It's quite simple but easily expandable in case it's needed.

@Neikon
Copy link
Contributor

Neikon commented Jan 25, 2021

Of course, it doesn't matter if it doesn't load locally, it's not made for that. it will always be viewed online, so there's no problem.

is there a way to view it online? i didn't find a way. if not, don't worry, just I activate that setting in the "about:config" and that's it.

Don't worry about "click to bigger preview" right now, if we are going to change it to a card system with a cleaner code. Then we'll make sure that it works.

@unseen-ninja
Copy link
Collaborator

I've uploaded it real quick. Here is a live demo.

@BeyondMagic
Copy link
Collaborator Author

A temporary fix would be to set privacy.file_unique_origin to true in your about:config..

Isn't false ? I think.

@andreasgrafen, the themes.json can be an array with objects (themes) inside, this way there's no need to add a new key name with the next number every time.

image

@unseen-ninja
Copy link
Collaborator

True! I've run it through an auto-converter and honestly just didn't think about it. An Array is the better solution by far though. c:

@unseen-ninja
Copy link
Collaborator

Jeebus Christ. This template does some weird things to the list entries…

The auto-loading itself works fine as it is right now. I've also updated the live preview.
What currently does not work is the popup functionality. For now I've temporarily linked the list entries directly to the corespoding repository. I'll try to figure out what the theme actually does in order to restore this functionality but it may take a litte time. c:

@BeyondMagic
Copy link
Collaborator Author

BeyondMagic commented Jan 27, 2021

I think most of the optimization is done, we can definitely rework in the scripts if (and only if) necessary, but right now, I don't think there's any problems of loading, as @andreasgrafen said, there's the popup functionality that doesn't work and it seems (at least for me) not very necessary, I like the way it looks right now, but open another issue if there's work to be done, since every theme has a description and would be better to put it on somehow.

And as a quick remark, re-open if we encounter problems in the future as we add more and more themes (since it will be loading more and more images).

More one thing, @Neikon, please, I'd like to participate on the project, so invite me as member to help you with the next issues.

No Sign up for free to join this conversation on GitHub. Already have an account? No Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants