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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

cintek
Copy link

@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").

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 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
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
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.

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.

None yet

3 participants