Skip to content
New issue

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

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

Modified Manga CSS Hover Styling #17

Merged
merged 12 commits into from
Oct 30, 2023
Merged

Modified Manga CSS Hover Styling #17

merged 12 commits into from
Oct 30, 2023

Conversation

db-2001
Copy link
Collaborator

@db-2001 db-2001 commented Oct 26, 2023

Hi Guys,

I'm new to GitHub and sort of new to coding, but I really want to learn more and support this project. I made a small visual tweak (that took me much longer to figure out than it should have) so that by default the beautiful manga posters are displayed on the website (since most of the time we can identify the manga by poster alone) and then the publication-information is shown once you hover over it.

image

If I've messed up something in the process or code then please lemme know, like I said, super new to collaborating on GitHub and stuff so any guidance is appreciated.

~Dity

@C9Glax
Copy link
Owner

C9Glax commented Oct 26, 2023

Hey there, glad that someone is willing to contribute so don't worry if there are mistakes.

I have one thing I would like you to reimplement: the background-gradient, so that the text would also be readable on light covers.

Also if possible: Would you be able to make this an opt-in option? I really like both versions, so if possible could you add a checkbox somewhere (for example in the settings) that makes this toggleable? If not that is also fine, just lmk!

Again thanks for contributing!

@C9Glax C9Glax marked this pull request as draft October 26, 2023 12:17
@C9Glax C9Glax self-requested a review October 26, 2023 12:17
@C9Glax C9Glax added the enhancement New feature or request label Oct 26, 2023
@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 26, 2023 via email

@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 26, 2023

I managed to get the gradient back in, before moving on to making it a toggleable setting, is there a preference for where the settings are stored? In the website container in a JSON file or having to make a request over the API for it? I would lean towards having website-specific settings staying in the tranga-website container in a file to reduce the communication overhead but I wanted to get somebody more experienced to give their views on it.

Also is there any reason why simply changing the style sheet would make the pop-up for the manga information summary not work? When I change to the default style sheet the pop up works fine and then when I use mine with the couple of CSS modifications (same file as above), the pop-up shows blank.

image

@C9Glax
Copy link
Owner

C9Glax commented Oct 26, 2023

I have no idea what I am doing with CSS/HTML/Javascript myself, so I don't have any preference on were to store settings.
The only current variable "Api-Uri" is stored in a cookie...

On why the popup would stop working:
For whatever reason I have also named that container publication-information. Now only god knows why I did that, but since there is no publication to hover over, the flex-setting is not properly changed...

So if you could kindly change the container of the viewer to something else and add

display: flex;
flex-direction: column;
justify-content: start;

to it's properties, that should fix it.

@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 26, 2023

Ah, yeah, that fixed it. I'll do a little bit more research and playing around with where to store the settings and let you know where I end up.

@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 27, 2023

Ok, so in the end I decided to go with storing the setting in the api container since the style sheet is something you probably want to set for each instance of tranga rather than every single browser window. I was more or less just copying/pasting what was already there for the other API calls but if you could double check that it looks like it's implemented correctly then that would be awesome.

Obviously, this means that the API code itself has to be updated, and I haven't forked that code so if you could help with that until I figure out how to run the API code locally (help with this appreciated), it would be much appreciated.

Another thing I noticed is that clicking the Update button in the settings window depopulates all of the manga titles and leaves only the blue add new manga button. This happens both in my code as well as the official docker release that I currently have running. This brought me to the question, where/how are the manga titles getting populated in the first place? From what little reverse engineering I could do, I came to the conclusion that it has something to do with the if statement in 'UpdateJobs()' in 'interaction.js' because on the first call/run, 'monitoringJobsCount' is set to zero and then after 'UpdateJobs()' has run, it's set to 'json.length' so then when it get run again by 'Setup()' which is called from 'UpdateSettings', the if-statement obviously doesn't evaluate to true.

I guess the way forward for this would be:

  1. Fix the if-statement logic to allow being run again
  2. Populate the manga somewhere/somehow else
  3. Avoid calling Setup() in UpdateSettings() - I'm not too sure why it's being called in the first place.

If you have any guidance on which way forward is best or why implementing one of the above options would fatally break something, or if there's another option, I'm all ears.

P.S. Looking forward a bit, I was hoping to implement a small box (green background, white text) in the bottom right corner of each manga that informs what the latest chapter number downloaded on disk is (in a separate pull request), I'm not sure if there's an API call that already does that or if it has to be implemented as a custom call. It wouldn't be able to simply count all the files in each folder because of all the Omake, 0.1/0.2/0.5 chapter that get released, so it would have to look for the highest chapter count number. I don't know how Tranga handles chapters that are added in between others (so like 123.5 added in between 123 and 124 but after 124 has been added to the website, this typically happens with the volume extras once the actual manga volumes come out), but if there's some way to check that there is a mismatch between the chapters on the connector and the chapters on disk, then the background of the chapter indicator could turn yellow.

@C9Glax
Copy link
Owner

C9Glax commented Oct 27, 2023

looks like it's implemented correctly

looks good, values would be hover, and default - correct?

I haven't forked that code so if you could help with that until I figure out how to run the API code locally (help with this appreciated)

If you don't already have an IDE I can recommend Jetbrains Rider, or just the default Microsoft Visual Studio for .NET development.

On the whole update-button situation:
While HTML and CSS is not my forte, JavaScript is my bane. I don't have the code present right now, but my guess would be, that I was calling Setup() to reset the website...
But yea I will open an issue for that, not really part of this 😄 #18

On the latest chapter number:
This has already been an idea, but not a concrete issue.
The problem with the orange number thing would be, that as soon as tranga checks that there is a new chapter, it automatically downloads it, pretty much making the feature useless...
The chapternumber returned is the whole string "123.5"

@dityb
Copy link

dityb commented Oct 27, 2023

Yes the values would be 'default' and 'hover'.

I've just been using VS Code for my IDE. I meant more of once I have a local copy of all the code I don't know how to actually go avout running it. The webpage I just open up in Firefox to check if what I did is working correctly.

As for the website breaking, it makes sense to refresh the website, I'll keep looking into what other ways to fix it but probably in a separate pull request just for organization.

The latest chapter number post you linked to is also actually me lol, just my school account which i didn't realize I was logged into earlier. The difference between that and what I proposed earlier in this thread is that returning the latest chapter number from each source would be primarily a feature for search, when initially selecting a new manga, and returning the latest chapter number that you currently have downloaded is just a check to make sure everything is working properly/check if the website you normally get stuff from is being updated. So you're pulling the latest chapter number from two different places: from the connector during search, and from what you have downloaded for the default home page. If that doesn't make sense I can try and elaborate more.

Also yeah, I hear you that changing the background color to orange/yellow isn't helpful since Tranga would just download it then. I think still having the green box for what is downloaded on disk is useful for in between scan intervals, whatever that is, whether it's one hour or two hours or something.

@C9Glax
Copy link
Owner

C9Glax commented Oct 27, 2023

I've just been using VS Code for my IDE. I meant more of once I have a local copy of all the code I don't know how to actually go avout running it. The webpage I just open up in Firefox to check if what I did is working correctly.

You might want a full-blown IDE with debugger. The IDE will automate tasks like building the project and also make error-searching easier, as you can actually look at objects and variables during runtime.

is also actually me lol

I was already wondering because it would show dityb in the email-notifications 😆

I now get what you mean, the goal. C9Glax/tranga#74 is the issue for that on the api side, I would add two fields to the Manga-Class. One that represents the latest downloaded chapter, and one that represents the latest available.

@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 27, 2023

You might want a full-blown IDE with debugger. The IDE will automate tasks like building the project and also make error-searching easier, as you can actually look at objects and variables during runtime.

Thanks! I'll definitely look into that.

For this feature, I'd like to pull the cutting-edge image on my production environment and test with the proposed code changes here before calling it "ready for review" so could you just let me know whenever you end up being able to implement that? No rush :). Also I noticed that the API has updated code for the latest downloaded chapter and available chapter so I'll get started on those once this is resolved. Is there a way for the latest downloaded chapter to check what is actually on disk rather than updating when a new one is downloaded? I ask because then once the changes go live, I believe it'll show 0 chapters for everything until a new chapter is downloaded - not really a problem in the long run but I'm sure it'll be alarming to some people since many don't read patch notes.

@C9Glax
Copy link
Owner

C9Glax commented Oct 27, 2023

I am gonna do you one better:
There is now a branch dev that automatically builds and pushes to docker.
Before we can merge however, you need to also merge index.html (please keep the new changes, and add another div).

Is there a way for the latest downloaded chapter to check what is actually on disk

https://github.com/C9Glax/tranga/blob/d78897eb745efe3f771e692409333fd2bddf02b1/Tranga/Manga.cs#L89C1-L89C1 Is already in a todo, just need to figure out a way that works properly... I might just unzip and get the property info from there, but we will see.

@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 27, 2023

Before we can merge however, you need to also merge index.html (please keep the new changes, and add another div).

I'm a little confused merge 'index.html' with what and add another div where?

Also I should have clarified, I wanted to test the new API calls so I wanted to know when the cuttingedge API branch gets updated to have those.

@C9Glax
Copy link
Owner

C9Glax commented Oct 27, 2023

Okay from the top:
Right now there is a merge conflict between what is currently on the master branch (because I changed it after your fork) and what you have on your PC. Therefor we need to resolve that conflict.
grafik
This is the conflict, because I also added a div with content to the website. I want you to combine what is on your PC and what is on the masterbranch. However the automatic resolve will probably put everything into one div, that's why I said

add another div

Also I should have clarified, I wanted to test the new API calls so I wanted to know when the cuttingedge API branch gets updated to have those.

It already is. If you pull the new docker-image from cuttingedge the calls should be available

Added an empty <div> to mitigate merge conflicts. Running GetSettings in Setup so that the webpage CSS is set up whenever the website runs for the first time and not just when the settings get opened. This shouldn't cause any issues even though setup is called a few times throughout the code.
@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 27, 2023

Ok, made a new commit with an empty <div> so let me know if that was enough. When I pulled the latest image, I got an HTTP Bad Request for posting the style sheet change. I'm currently rebooting my server (it was being slow for unrelated reasons) and I'll try again once the stack redeploys.

…e default.

If the styleSheet property doesn't exist or isn't equal to a valid string, it used to default to the mangahover.css from the else statement. Logically, that means that mangahover was the default instead of the actual default. This has been rectified.
@C9Glax
Copy link
Owner

C9Glax commented Oct 27, 2023

Okay not quite what I had in mind... 😄

What I wanted you to do:
On your machine merge this remote master branch to your working one.
So pull the master branch from this repo (https://stackoverflow.com/questions/1783405/how-do-i-check-out-a-remote-git-branch), switch to your branch git checkout your-branch, and run git merge master if the master-branch is called that on your machine.
You can also just go to your github-fork of this repo, and you should see a "... is x commits ahead" message.
You then only have to pull your branch from your repo.

It should give you a merge conflict (the one above). You can use a graphical editor and "keep both", or you can simply delete what you don't want to keep in the file. In the end it should look like this:

            <div>
              <span class="title">LunaSea</span>
              <div>Configured: <span id="lunaseaConfigured">✅❌</span></div>
              <label for="lunaseaWebhook"></label><input placeholder="device/:id or user/:id" id="lunaseaWebhook" type="text">
            </div>
            <div>
              <input type="checkbox" id="mangaHoverCheckbox" name="css-style" value="style_mangahover.css">
              <label for="css-style"> Show manga titles and sources on hover</label><br>
              <span class="title">Ntfy</span>
            </div>
            <div>
              <div>Configured: <span id="ntfyConfigured">✅❌</span></div>
              <label for="ntfyEndpoint"></label><input placeholder="URL" id="ntfyEndpoint" type="text">
              <label for="ntfyAuth"></label><input placeholder="Auth" id="ntfyAuth" type="text">
            </div>
            <div>
              <input type="submit" value="Update" onclick="UpdateSettings()">
            </div>
          </popup-content>

@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 27, 2023

I think I did it? Thank you for being patient while I figure this stuff out.

HTTP Bad Request for posting the style sheet change.

Looks like when I print the Settings Json out to console, the styleSheet property isn't there.

C9Glax added a commit to C9Glax/tranga that referenced this pull request Oct 28, 2023
@C9Glax
Copy link
Owner

C9Glax commented Oct 28, 2023

I think I did it?

You did! 👍🏼

Looks like when I print the Settings Json out to console, the styleSheet property isn't there.

Because I didn't add it yet :) Should be there now

@db-2001
Copy link
Collaborator Author

db-2001 commented Oct 30, 2023

Awesome, when I merged with the main branch there seemed to be a couple things that broke, but they should be fixed now. Feature works as intended:

Default behavior:
image

What default behavior looks like in settings:
image

Changing to the new behavior in settings:
image

What the new behavior looks like:
image

@db-2001 db-2001 marked this pull request as ready for review October 30, 2023 02:26
@C9Glax C9Glax merged commit e01683d into C9Glax:master Oct 30, 2023
@C9Glax
Copy link
Owner

C9Glax commented Oct 30, 2023

Looks tasty :)

@db-2001 db-2001 deleted the dityb branch October 30, 2023 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants