scooterpsu / Comixology_Ubooquity_2

38 stars 7 forks source link

cleaning up old lines from v1 publisher html #27

Closed CuddleBear92 closed 4 years ago

CuddleBear92 commented 4 years ago

Whats the state of the two script src lines and the userinfo2 lines? Are they used at all in the v2 version of the theme?

Noticed one of the js files was in \comixology2\js\jquery-3.3.1.min.js

So i guess that one atleast wants to be used, editing the line to ../../js/jquery-3.3.1.min.js would be the correct thing to do im guessing.

Also guessing the <div id="userinfo2"></div> line is not needed atleast anymore.

Please help me correcting these with how they should be.

explorer_n3J4jT4iWy

Notice in your example that they are not used at all, so they don't do anything anymore then? Even when they are in the theme itself?

Also noticed the <script>document.getElementById("group").classList.add("publisherPage");</script>

Is that important in anyway?

scooterpsu commented 4 years ago

That's not how that works. They don't need to be added to html files you create because they're added to every page using themeScript.js. I specifically cut down the user-created files to bare minimum.

And yes, that add of the publisherPage class is a fallback in case the theme doesn't pick up on what type of page the html is supposed to be. Different css is applied using that class, that's what classes are for.

CuddleBear92 commented 4 years ago

Alright, thanks for clearing it up, removing the two js lines and the userinfo2 line and adding that publisher page line at the end as in your example across my repo then! Sounds good!

CuddleBear92 commented 4 years ago

so i guess something like this looks good?

<link rel="stylesheet" type="text/css" href="[[FOLDER]]/folder.css">
<div align="center"><img src="[[FOLDER]]/header.jpg" width="100%"></div>
<div class="imprintNav" style="center">
    <ul>
        <li><a class="active" href="/ubooquity/comics/1626/">Marvel Comics</a></li>
        <li><a href="/ubooquity/comics/49340/">Marvel Knights</a></li>
        <li><a href="/ubooquity/comics/49186/">Max</a></li>
        <li><a href="/ubooquity/comics/49056/">Icon</a></li>
        <li><a href="/ubooquity/comics/1626/">Epic</a></li>
    </ul>
</div>
<script>document.getElementById"group".classList.add"publisherPage";</script>

Have already done these changes on my repo across the board. No zip release yet though.

scooterpsu commented 4 years ago

Still works best including the breadcrumb line,

<link rel="stylesheet" type="text/css" href="[[FOLDER]]/folder.css">
<img id="pubHeaderImg" src="[[FOLDER]]/header.jpg">
<div class="imprintNav">
    <ul>
        <li><a href="/ubooquity/comics/2336/">DC Comics</a></li>
        <li><a class="active" href="/ubooquity/comics/152/">Vertigo</a></li>
        <li><a href="/ubooquity/comics/162438/">WildStorm</a></li>
    </ul>
</div>
-->  <div class="breadcrumb" id="cmx_breadcrumb"><a href="../">Comics</a> > <a href="../">Publishers</a> > Vertigo</div>
<script>document.getElementById("group").classList.add("publisherPage");</script>
CuddleBear92 commented 4 years ago

yeah, was aware of it. Don't like it as its a hardcoded step and it is automatically using the theme default.

I could always make a wiki guide for users wanting it back in.

Also the white background of it normally is also messing up the clean look of the theme i feel. And if i where to add CSS to that again, i guess that would be another complication.

Yes it doesn't look As nice cause there is a small glip and the top of the page for the breadcrumb to place it self.

Don't think it looks bad with the auto-resolve though.

Random thought! Moving it to the top of the screen next to the search bar could be a thing, that way its not as destructive to the looks while still having the option for it.

Not sure how to handle mobile widths with that though.

brave_v51dSS2ddj

scooterpsu commented 4 years ago

I mean the background of the breadcrumb is going to be the same, regardless of location. Unless you're carrying over more excessive CSS. My bad, the white is intentional to match the Comixology style, but it's blue on the dark themes.

And yeah, the auto generated one works, but it cuts down on the necessity if you used the hardcoded. A page will load faster if less needs to be generated on the fly. Plus including it in the html moves it to the location on an actual Comixology publisher page.

As for moving it to the top bar, that would take a lot of work and move away from the goal of recreating the Comixology page style. You're welcome to take a crack at it though.

Overall this theme is pretty terrible for mobile, as I've only copied Comixology's desktop CSS. That's why I specifically direct people to use better optimized apps for a better mobile experience.

CuddleBear92 commented 4 years ago

Ahhh, didn't know about the faster loading, thanks for informing me on that. Things doesn't load slow here generally locally.

Personally wouldn't be that turned off by the breadcrumbs if it used the 3 base darkmodes.

Thinks the theme works great for iPads at a minimum, a bit bad on phones since the screens are smaller though.

But this is shaping up more and more now, with publisher and series metadata access on the ease from the scraping script.

The last big thing is automation on story-arcs, book series and authors. Which is a requirement i think going forward.

scooterpsu commented 4 years ago

The breadcrumbs on publisher pages do work with the base dark modes, they turn blue. It just matters where the breadcrumb is, and if that line adding the publisherPage class is loaded.

https://i.imgur.com/xKNFzHI.png

CuddleBear92 commented 4 years ago

mhm, aware. don't notice anything on the speed and performance here with ctrl+f5 refreshing between it. hmm I guess there is some sort of cache going on too somewhere?

Debating the worth of filling it. Maybe i can get someone to make a quick script that took the folder name and added that line at the second last line.

or add it to the scraping script and re-running it... took 2 days to run that one though.

scooterpsu commented 4 years ago

It's caching Ubooquity IDs to localStorage, so that's likely what's keeping things moving quickly. The on-the-fly breadcrumb generation used to take a lot of time before that was implemented, it might be negligible now.

CuddleBear92 commented 4 years ago

Yeah, think i wont do it with the first. Atleast not push for it atm. Gonna try to edit the script and see if it still runs though, shouldn't be an issue i think. So atleast if people like to run the scrape themselfs for whatever reason can get it then. Guess ill replace it over time and do another scraping run.