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

Improve modify layout #1525

Merged
merged 2 commits into from
Sep 14, 2024
Merged

Conversation

cintek
Copy link
Contributor

@cintek cintek commented Sep 25, 2023

In most forms the buttons to confirm or cancel are at the bottom because in western countries we read from top to bottom. Because of that this PR moves the buttons for confirmation, cancellation and preview to the bottom. It also changed the order of the buttons because usually a button on the right lets users do anything to go on and a button on the left lets them go back. It also has to do something with how we read (from left to right; the right site is the "future").

Only the button to recover the draft stays at the top since it should be near the message which tells you that you can recover a draft.

The next change is that I also moved the alerts to the bottom because the feedback has to be where the buttons for interaction are. (This causes problems, see discussion below)

Furthermore, this PR removes the General meta header because comments are also meta data.

This PR affects at least the basic and the modernized theme.

improve-modify-layout

@RogerHaase
Copy link
Member

Moving the "You may recover your draft ..." flash message to the bottom makes it less useful in that some/most editors would not realize a draft was available until they finish re-editing and scroll to the bottom to click the save button.

You could also delete the General meta title. Comment, Summary, and Tags are all meta data.

@cintek
Copy link
Contributor Author

cintek commented Oct 2, 2023

Moving the "You may recover your draft ..." flash message to the bottom makes it less useful in that some/most editors would not realize a draft was available until they finish re-editing and scroll to the bottom to click the save button.

In this case it really makes sense to show the flash on top of the page. I will have to take a look how it's possible to use the flash in two different places: On at the top and one where the buttons are.

You could also delete the General meta title. Comment, Summary, and Tags are all meta data.

Alright

@cintek
Copy link
Contributor Author

cintek commented Oct 2, 2023

I'm not sure what the best way is to solve this. Here is an idea:

We could use a HTML element at the top and at the bottom (where the buttons are) with class moin-flash. At the top we would show only messages like "You may recover your draft ..." by using if states. At the bottom we would show the rest of the messages, also by using if states.

This would only work if the text was not translated at that point but I'm not sure if that is the case.

A disadvantage is that at everytime someone changes the text of the message this part has also to be updated.

This example is for illustration of the idea:

<div id="top">
    <div class="moin-flash">
        {% for category, msg in get_flashed_messages(with_categories=true) %}
            {% if msg == "You may recover your draft ..." %}
            <p class="moin-flash moin-flash-{{ category }}">{{ msg }}</p>
            {% endif %}
        {% endfor %}
    </div>
</div>

<div id="bottom">
    <div class="moin-flash">
        {% for category, msg in get_flashed_messages(with_categories=true) %}
            {% if msg != "You may recover your draft ..." %}
            <p class="moin-flash moin-flash-{{ category }}">{{ msg }}</p>
            {% endif %}
        {% endfor %}
    </div>
</div>

@UlrichB22
Copy link
Collaborator

This would only work if the text was not translated at that point but I'm not sure if that is the case.

A disadvantage is that at everytime someone changes the text of the message this part has also to be updated.

You can try the categories feature of flashing, see https://tedboy.github.io/flask/patterns/flashing.html#flashing-with-categories. When you change the category to 'info-top' you can filter it in the template, e.g.:

<p class="moin-flash moin-flash-info">Flash messages:</p>
{% for category, msg in get_flashed_messages(with_categories=true) %}
    {% if category == "info-top" %}
        <p class="moin-flash moin-flash-info">{{ msg }}</p>
    {% endif %}
{% endfor %}

By the way, I am preparing a change regarding translations, which could also change some text phrases. IMO, using the msg text in an if statement is not an option.

@cintek cintek force-pushed the improve-modify-layout branch 2 times, most recently from 9218f16 to 6a1f090 Compare September 12, 2024 08:09
Not necessary as e.g. comment is also meta data.
@cintek
Copy link
Contributor Author

cintek commented Sep 12, 2024

Sorry for the huge delay. I had much to do in our projects.

Thank you @UlrichB22 for your answer. I decided to let the flash stay where it is for now. As @RogerHaase explained it is no good idea to show the flash to recover the draft at the bottom. And most times a page is freshly loaded before you see a message so it's better to keep the flash at the top.

I will edit the description of the PR to reflect the changes that are now done by it.

BTW: You could even filter the categories which would make your solution even shorter, @UlrichB22 ;)

@RogerHaase RogerHaase merged commit 8fe0d22 into moinwiki:master Sep 14, 2024
6 checks passed
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.

3 participants