Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

#7676 - Flash messages#7684

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
ghost wants to merge4 commits intosymfony:masterfromunknown repository
Closed

#7676 - Flash messages#7684

ghost wants to merge4 commits intosymfony:masterfromunknown repository

Conversation

@ghost
Copy link

@ghostghost commentedMar 25, 2017
edited by ghost
Loading

This pull requestfixes#7676 that documents the flash messages that are been changed insymfony/symfony#21819.

@linaori
Copy link
Contributor

In the context of the text I have some remarks:

In the template of the next page (or even better, in your base layout template), read any flash messages from the session:
- controller.rst

For example, one common problem in this situation involves checking for flash messages, which are stored in the session. The following code would guarantee that a session isalways started:
- avoid_session_start.rst

Minor details, but both of them are still referring to using the session, while the examples do not use the session directly anymore (no reference toapp.session). Maybe those 2 pages could do with a note that this is a (new) shortcut?

@wouterj
Copy link
Member

Besides@iltar's comment, we should also add a versionadded directive after the code example:

..versionadded::3.3    The `app.flashes()` method was introduced in Symfony 3.3. Prior to version 3,3    you had to use `app.app.session.flashBag`.

@wouterj
Copy link
Member

Status: needs work

@javiereguiluz
Copy link
Member

javiereguiluz commentedMar 25, 2017
edited
Loading

We should also deprecate or refactorize or add a big warning message to this article:https://symfony.com/doc/current/session/avoid_session_start.html

@ghost
Copy link
Author

@javiereguiluz I think we should deprecate it because your new method does not longer checks the session making that part of the documentation confusing and not longer needed.

<?php endforeach ?>

..versionadded::3.3
The `app.flashes()` method was introduced in Symfony 3.3. Prior to version 3.3

Choose a reason for hiding this comment

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

Unlike Markdown, in RST files we need to use double-backticks for code:

``app.flashes()``

Copy link
Author

Choose a reason for hiding this comment

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

@wouterj
Copy link
Member

hasPreviousSession() still can be use-full imo. So I would keep the article by adding a small.. tip:: saying that there is this new niceflashes() method that can be used instead. We can always update the example if we can come up with a better one in the future.

Regarding the text, I think we can leave the session bit in there and do something like:"In the template of the next page (or even better, in your base layout template), read any flash messages from the session usingapp.flashes():" Not 100% sure though

linaori reacted with thumbs up emoji

@ghost
Copy link
Author

By the thumbs up from@iltar I assumed the suggestion from@wouterj seems to be correct.

Regarding yourhasPreviousSession() it is still useful but that makes these lines (source file) not needed imo. Should I replace them with the small.. tip:: Since Symfony 3.3 there is a better method to read the flash messages from the session. See controller.rst#flashmessages... something like that?

Copy link
Contributor

@HeahDudeHeahDude left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@javiereguiluzjaviereguiluz left a comment

Choose a reason for hiding this comment

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

👍

@xabbuh
Copy link
Member

Thank you @Ricknox.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improved flash messages handling in Twig templates

7 participants

@linaori@wouterj@javiereguiluz@xabbuh@HeahDude@carsonbot@ricardodevries

[8]ページ先頭

©2009-2025 Movatter.jp