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

Better UI: fix page title #69

Open
benoit74 opened this issue Oct 19, 2023 · 16 comments
Open

Better UI: fix page title #69

benoit74 opened this issue Oct 19, 2023 · 16 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@benoit74
Copy link
Collaborator

With current code of the new UI, on topic pages, the title is "Kolibri channel in ZIM".
On content pages, it is the content title, but it sometimes fails to be updated (or compete with kiwix-serve iframe code more exactly).

For simplicity / effectiveness, we should fix this to always have a constant title per ZIM which would be the ZIM title (and it needs to be static, i.e. not computed/manipulated by Vue.JS).

@benoit74 benoit74 mentioned this issue Oct 19, 2023
@benoit74 benoit74 added the enhancement New feature or request label Oct 19, 2023
@benoit74 benoit74 added this to the 1.2.0 milestone Oct 20, 2023
@benoit74 benoit74 modified the milestones: 1.2.0, 1.3.0 Feb 14, 2024
@Pagla-Dasu
Copy link
Contributor

Pagla-Dasu commented Feb 22, 2024

@benoit74 , Hi, I am trying to run the zimui locally, but this part of the command is not working and I have done extensive research and still didn't find any solution to this.

docker run -it --rm -v $(pwd)/output:/data ghcr.io/openzim/zim-tools:latest zimdump dump --dir=/data/Minimal_Test /data/Minimal_Test.zim

The output to this command says, docker: invalid reference format.
Can you guide me a little about the solution if there is any.

p.s - I am trying to solve this issue, so you can assign me this

@benoit74
Copy link
Collaborator Author

This is a local developer issue, unfortunately we cannot assist all developers with problems on their machine setup.

Hint: https://stackoverflow.com/questions/45682010/docker-invalid-reference-format

@Pagla-Dasu
Copy link
Contributor

@benoit74 , sorry for the inconvinience, solved the docker issue and the ui is working perfectly, starting to work on the current issue present.

@Pagla-Dasu
Copy link
Contributor

@benoit74 , just looking for some clarifications here, do you want the title to be "ZIM" or do you want the title to be the name of the zim folder that was scrapped by docker? Eg - "Minimal Test"

@benoit74
Copy link
Collaborator Author

Good question, thank you, we want it to be the --title parameter, as stored in self.title of Kolibri2Zim class.

You will probably have to pass it somehow to the Vue.JS part. One solution would be to add this information to the new channel.json through the Channel class, but then it means the page title is again dynamically loaded by JS, and it proved to not work very well (as mentioned in original comment). Another solution probably has to be found (maybe the problem was only that current code is setting the title too late and the channel.json should simply be loaded way earlier, closer to Vue.JS application initialization).

@benoit74
Copy link
Collaborator Author

As mentioned in first comment, it is important to test the final behavior in the ZIM, with a kiwix-serve server serving the ZIM file.

@ArnabBCA
Copy link

@benoit74 , sorry for the inconvinience, solved the docker issue and the ui is working perfectly, starting to work on the current issue present.

@Pagla-Dasu Hi, can you please tell me the steps which you followed to set up the project locally. I am having some trouble setting it up.
Thanks.

@Pagla-Dasu
Copy link
Contributor

@ArnabBCA Hey, so, go to the "new_ui" branch and then go to the CONTRIBUTING.md file and follow the steps written on it. Make sure to have the Docker Deamon running while you run the docker run steps. Just follow the documentation and you will be good. If you can any issue feel free to communicate.

@ArnabBCA
Copy link

@ArnabBCA Hey, so, go to the "new_ui" branch and then go to the CONTRIBUTING.md file and follow the steps written on it. Make sure to have the Docker Deamon running while you run the docker run steps. Just follow the documentation and you will be good. If you can any issue feel free to communicate.

Thanks, also did you set it up in linux or windows?

@Pagla-Dasu
Copy link
Contributor

Thanks, also did you set it up in linux or windows?

Unix(Mac)

@benoit74 benoit74 modified the milestones: 1.3.0, 2.0.0 Mar 27, 2024
@Sudarshan-21
Copy link
Contributor

@benoit74 can you attach the relevant screenshot and/or channel-id to reproduce this issue

@benoit74
Copy link
Collaborator Author

benoit74 commented Apr 8, 2024

All channels are impacted. The problem is the HTML page title, the one present in <html><head><title> or tweaked through JS code. This is the title displayed in browser tab typically.

@Sudarshan-21
Copy link
Contributor

Sudarshan-21 commented Apr 10, 2024

Screenshot from 2024-04-10 20-20-34

@benoit74 the display property for the title is set to none. So, why exactly are we worrying about it when we are not displaying it anyway? (I have read the main comment but my question still stands).

@benoit74
Copy link
Collaborator Author

the display property for the title is set to none

This is your user-agent which does this, not our code ; anyway, <head> data is never displayed in the web page.

So, why exactly are we worrying about it when we are not displaying it anyway?

It is displayed in your screenshot (see purple circle):

image

And we do not want:

  • this generic title on topic pages
  • the content title on content pages (PDF, ePub, HTML5 app, video) - sometimes, this is buggy anyway

We want to always have the ZIM title.

@Sudarshan-21
Copy link
Contributor

Sudarshan-21 commented Apr 14, 2024

I have been working on this issue for the past few days, I am still figuring out the complete fix for this issue. This one seemed pretty easy at first, but it isn't.
I am writing this comment for the fellows who are/will be working on this issue to get a kickstart in understanding the issue and where exactly to look at:
So the explanation of the issue is already given in the main comment, and what needs to be fixed and why is mentioned in this comment #69 (comment).

As far as I have understood:
The issue is with setting the <title>A Kolibri channel in zim</title> in the template to a statically calculated value of title which will look like <title>{{title}}</title>. This value of the title is to be set to the self.title. You will find this self.title in the kolibri2zim class inside the scrapper.py file.

What I have tried already:
My first thought was to use the jinja2 and get the index.html template into Scrapper to modify it. but the problem with it is that index.html is not in the templates folder(all the HTML files in this folder can be accessed to be used by jinja2 as templates because they all lie in the same parent folder as of the scrapper and they are already getting accessed by jinja2 using the loader eg. html = self.jinja2_env.get_template("audio.html").render() )and I couldn't access the path to index.html using the resolve().parent.parent method(I am not sure why but it wasn't able to resolve the path. I tried various combinations of the methods but kept getting errors similar to [Errno 2] No such file or directory: '/usr/local/lib/python3.12/zimui/index.html' the path is /home/sudarshan/kolibri/zimui/index.html ).
Then I tried giving the relative path using ./Path_to_index.html now it was able to read the file, but the title in index.html is not getting the value from self.title. It remained the placeholder text ie. {{title}}. I am looking forward if I can fetch this title using AJAX into the HTML file.

I might be wrong somewhere, feel free to correct.

@benoit74 benoit74 assigned benoit74 and unassigned Pagla-Dasu Sep 24, 2024
@benoit74
Copy link
Collaborator Author

This has been one quite neatly in youtube:

  • a generic title in index.html from Vue.JS as fallback
  • a dynamic title in vue code to adapt to the vue context

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

No branches or pull requests

4 participants