thuiop / magium-dev

MIT License
13 stars 4 forks source link

[Discussion] Cleaner code #45

Open ZiClaud opened 2 months ago

ZiClaud commented 2 months ago

Most logic needs to change, I'm changing some stuff with the .css but I can't understand how stuff like the header works.

The worst thing is that there's no clear distinction from back-end and front-end, I kinda feel like we should clean the code before continuing, maybe put some rules.

For example, at the end of most of the .ejs there is this:

<div style="display:none">
    <h2 id="header" hx-swap-oob="true">
        <%= id %>
    </h2>
</div>

Why is it at the end if it's a header? And what does it do, exactly? Why is there "style" on the .ejs?

I'd put many .css, stuff like header.css, footer.css, text.css, etc. Instead of having them all inside main.css


Something cleaner would be:

<div class="header">
    <h2>
        Book <%= book_id %> - Chapter <%= chapter_id %>
    </h2>
</div>

So that if we want to change stuff it's easier, like switch from that to:

<div class="header">
    <h2>
        Book <%= book_id %>
    </h2>
    <h3>
        Chapter <%= chapter_id %>
    </h3>
</div>

(Honestly idk if we the stuff inside <%= %> is possible or completely wrong, let me know)

thuiop commented 2 months ago

Yes, there is a lot of stuff to clean indeed.

Why is it at the end if it's a header? And what does it do, exactly? Why is there "style" on the .ejs?

This is an out-of-band swap. When pages are loaded, everything within the div #content is swapped by the response of the server. However, we also want to change the header, and the header is not inside #content. Therefore, we use the hx-swap-oob tag, which will swap #header with the content of the div. The style is there because this is not an element meant to be rendered in any case; in fact it is typically deleted by HTMX. It used to be visible when loading the page for the first time, hence the display:none, but I think it is not anymore (does not hurt to keep it anyway).

Header appears where it shouldn't (on the Menu and Stats page).

What do you mean? I cannot see why the header should not appear on those pages?

Same thing with footer, I'm currently removing it from everywhere except the menu, and I needed to change way more than I wanted to.

Not sure why you want to avoid the footer either, but it is really has simple as moving the div within the one for the menu (and probably changing the styling a bit)

The worst thing is that there's no clear distinction from back-end and front-end

I am not sure what you mean there. When moving to a new page or fetching new content, the front-end makes a query; that is all there is to it.

ZiClaud commented 2 months ago

What do you mean? I cannot see why the header should not appear on those pages?

I mean, it doesn't appear on the app, also I always try to click "Main menu" again to leave the menu lol

Not sure why you want to avoid the footer either

I want to remove it since it appears only on the menu on the main app, also to give it a bit of a cleaner look

but it is really has simple as moving the div within the one for the menu (and probably changing the styling a bit)

It's not, since moving it will give it padding, since the padding was added to all the content, not to the single elements, working on fixing it

I am not sure what you mean there. When moving to a new page or fetching new content, the front-end makes a query; that is all there is to it.

Sorry if I didn't explain myself well, but stuff that's in files like stats.js:

<% if (locals.v_magical_power) { %> <%= v_magical_power %> <% } else {%> 0 <% } %>

and

<% function sceneAfter(id) {
    const regex = /(B(?<book>[0-9]*)-)?Ch(?<chapter>[0-9]*)[a-c]-.*$/
    if (result=regex.exec(id)) {
        return (result.groups["book"] == "3") && (parseInt(result.groups["chapter"]) >= 4)
    }
} %>
<% if (sceneAfter(locals.v_current_scene)) { %>

just feel wrong to see in the middle of html, I feel like there should be a cleaner way to do it.

thuiop commented 2 months ago

Sure, but this is a web app, we do not have to copy the original 1 to 1. Having the header always visible allows you to switch directly from the menu to the stats for instance, which I think is better for navigation. The footer does not do much though.

just feel wrong to see in the middle of html, I feel like there should be a cleaner way to do it

This is how templating works though? It can definitely be made cleaner but this is regular stuff.

ZiClaud commented 2 months ago

If that can be made cleaner that'd be perfect