Skip to content

New theme 'focus' #1528

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

Merged
merged 1 commit into from
Apr 23, 2025
Merged

New theme 'focus' #1528

merged 1 commit into from
Apr 23, 2025

Conversation

cintek
Copy link
Contributor

@cintek cintek commented Sep 26, 2023

As mentioned on the mailing list I would like to contribute a new theme to Moin2.

This commit shows the changes to layout.html.

Screenshot 1: Sidebar and top bar

new_theme_1

Screenshot 2: Footer

new_theme_2

@cintek
Copy link
Contributor Author

cintek commented Sep 26, 2023

I have a question: How can I create a util.html that uses the util.html from templates but redefines some macros? I wasn't able to import my own file so I wrote the changed macros right into the layout.html.

@cintek
Copy link
Contributor Author

cintek commented Sep 27, 2023

With the last commit I changed the appearance of the modify window for the new theme:

Modify section in new theme

@RogerHaase
Copy link
Member

cintek, please make your moin repo public. It is easier to clone your repo and test all your changes than it is to review and download each diff separately.

@wagner-intevation
Copy link
Contributor

cintek, please make your moin repo public. It is easier to clone your repo and test all your changes than it is to review and download each diff separately.

The repo is at https://github.com/cintek/moin/tree/new-theme

@UlrichB22 UlrichB22 changed the title New Theme New theme 'focus' Nov 20, 2023
@cintek cintek marked this pull request as ready for review December 13, 2023 08:23
@cintek
Copy link
Contributor Author

cintek commented Dec 13, 2023

I marked this PR as ready for preview since now the most important work is done and it would be great to read what you think about this theme. Does is fulfill your expectations for a theme?

@UlrichB22
Copy link
Collaborator

Thanks for providing the new theme.
I found some minor issues

  • on the +index page the buttons are too small for the labels:

moin_theme_focus_index

  • the name of the theme should be extended to 'Focus Theme'. Then it fits to the other themes in the user settings dialog under Appearance.
  • Can you change the PR to only contain one commit?

@RogerHaase
Copy link
Member

Please add your theme to: http://moinmo.in/ThemeMarket

Also check the moin2 docs to verify there is a pointer to the above theme market, and the moin2 docs have working instructions for installing a contributed theme.

@wagner-intevation
Copy link
Contributor

Please add your theme to: http://moinmo.in/ThemeMarket

Do you mean by that that the theme mustn't be part of the moin repository, but in a separate repository?

@RogerHaase
Copy link
Member

RogerHaase commented Dec 18, 2023 via email

@wagner-intevation
Copy link
Contributor

Thank you for the answer.

Can themes add new translations?

@RogerHaase
Copy link
Member

RogerHaase commented Dec 18, 2023 via email

@wagner-intevation
Copy link
Contributor

@cintek
Do you think it is enough to instead of the current proposal just display a small triangle indicating a small triangle (First is the current version of the theme, the latter is without the new translation)? I don't think it is user-friendly enough and I think we need some good idea to solve that.

image

image

@UlrichB22
Copy link
Collaborator

It should be a rare case that additional translations are required for a new theme. Maybe we can add a dummy function somewhere and include the string 'More'. For javascript there is a similar workaround with src/moin/templates/dictionary.js.

I agree that a triangle without a label is not user-friendly.

@RogerHaase
Copy link
Member

I finally understand issue. Focus theme invents a new word or phrase that needs translation, but contributed themes may not be present when python setup.py extract_messages is run so translation is not possible.

Maybe add a dummy procedure to end of /themes/__init__.py

def contributed_themes():
    """Contributed themes may add translation strings here"""
    Focus_theme = _("More")

@cintek
Copy link
Contributor Author

cintek commented Jan 2, 2024

We have at least four options (I add one):

  1. Use only the triangle: I agree that this is not very user-friendly (especially because its such a small icon)
  2. Use only one icon but a different (bigger) one
    • Pro: Better than 1. because easier to see and to hit with the cursor
    • Con: Still no text
  3. Do the translation via JavaScript
    • Pro: Everything belonging to the theme is in one place
    • Con: Needs JavaScript enabled
  4. Use the function @RogerHaase mentioned: What I don't like about this solution is that the code for a contributed theme is distributed to different places - The theme with a separate repository (because it should be offered via the Theme Market) and the function which will part of the moin repository

I tend towards option 3 although it would be better if developers could just add a JSON file or something similar for translations.

@RogerHaase
Copy link
Member

A fifth option is to just use _("More") in the theme code and ignore that a tiny part of the theme is English only for now. If the theme is added to the main repo at some future date, the problem is self-correcting.

I do not understand the Javascript solution - how would that work?

@cintek
Copy link
Contributor Author

cintek commented Jan 3, 2024

Here is an example of how it could work. This way you would just need to add the attribute data-i18n to an HTML element and add a translation to the translations.js file. Of course this is a bit over-powered for just one static string so in our case I would write a shorter variant :)

@bernhardreiter
Copy link

Two remarks:

  • We (with @cintek and @wagner-intevation ) aim to have a theme which can replace the standard default theme (because we found it lacking, so in a sense it is not about style but a usability fix to the standard interface).
  • For the translation the function seems the best solution (from what I've read).

@cintek
Copy link
Contributor Author

cintek commented Apr 15, 2025

Yes, for now. We should concentrate on fixing/improving existing code rather than adding to the code base. Moin1.x has many contributed themes, moin 2.x should support contributed themes as well.

@RogerHaase So, when you've released a stable moin2 would it be an option for you to integrate the new theme into the repo?

Like @bernhardreiter wrote the most important point is that we want to improve the usability and that's what we did with this theme. It would be great if other users could benefit from it.

If it is okay for you to integrate the theme is there anything we should keep in mind? We've tested it already with some people and of course you will also test it. Some git conflicts have to be resolved. But is there anything besides these aspects to think?

@bernhardreiter
Copy link

To add: The feedback we've got is that our theme worked much better. Note that a theme is also code to me, as it is part of the overall experience. To give an example: A great engine without human interface would be useless. I'd say that the theme is even more important than some backend features.

@RogerHaase
Copy link
Member

Please add your theme to: https://moinmo.in/ThemeMarket.

@bernhardreiter
Copy link

bernhardreiter commented Apr 17, 2025

Please add your theme to: https://moinmo.in/ThemeMarket.

We can of course do that, but does that mean you will not accept a change of the standard theme to something more usable?

@RogerHaase
Copy link
Member

Long ago it was decided to work on fixing existing themes rather than create a quadrillion new themes with short lived support by the original authors. But somehow we ended up working on this new theme and now we have a pretty nice theme and the question is what to do with it. My answer is to make it a contributed theme. And maybe reevaluate in the future after fixing the existing themes to be less "lacking".

Maybe some other developer has a better idea?

@UlrichB22
Copy link
Collaborator

I think we should add this theme and include it in the delivery.

@UlrichB22
Copy link
Collaborator

@cintek, can you please apply the changes from #1780 to itemview.html and show.html in the focus templates dir?

@bernhardreiter
Copy link

Long ago it was decided to work on fixing existing themes rather than create a quadrillion new themes with short lived support by the original authors.

That is a concern I can fully understand. And it would make complete sense if there were one or several good default themes.

Our question is: How do we get one good default theme? Because no existing one worked for us - so far. That was our motivation to develop the new theme. If there is a different way to a good default theme, that would be fine. But if this is the best theme there is it can be a great improvement to Moin2's user experience.

It is true, we cannot promise that we will maintain the theme. We do have one small wiki installation in production where we use it and will use it for at least 2 years - so we will not drop it right now. Putting in more maintenance will depend on if we decide to move more old or new wikis to Moin2. This decision is still open, for us the current lack of speed (apparently as result of some design decisions like becoming a desktop wiki - which we do not need at all) is one blocker, so we will look at other wiki solutions as well. (This is just the current situation, feedback or questions should not be answered in this issue but somewhere else.)

@cintek
Copy link
Contributor Author

cintek commented Apr 22, 2025

@cintek, can you please apply the changes from #1780 to itemview.html and show.html in the focus templates dir?

Yes, I did and I finally followed your request to squash the commits.

Copy link
Collaborator

@UlrichB22 UlrichB22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your updates. For me, it is ready to be merged.

@UlrichB22 UlrichB22 requested a review from RogerHaase April 22, 2025 19:13
@RogerHaase RogerHaase merged commit b815152 into moinwiki:master Apr 23, 2025
8 checks passed
@cintek cintek mentioned this pull request May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants