[Admin] Flashes helper and reorganization #6280
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6280 +/- ##
==========================================
+ Coverage 87.99% 89.23% +1.23%
==========================================
Files 796 955 +159
Lines 16645 20079 +3434
==========================================
+ Hits 14647 17917 +3270
- Misses 1998 2162 +164 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
chaimann
force-pushed
the
admin-flash-helper
branch
from
fcb6fde to
30f7c7a
Compare
This was referenced Jun 6, 2025
Draft
chaimann
force-pushed
the
admin-flash-helper
branch
3 times, most recently
from
fdb205d to
85ab367
Compare
chaimann
added a commit
to chaimann/solidus
that referenced
this pull request
Jun 16, 2025
chaimann
added a commit
to chaimann/solidus
that referenced
this pull request
Jun 16, 2025
chaimann
added a commit
to chaimann/solidus
that referenced
this pull request
Jun 16, 2025
chaimann
added a commit
to chaimann/solidus
that referenced
this pull request
Jun 16, 2025
chaimann
added a commit
to chaimann/solidus
that referenced
this pull request
Jun 17, 2025
chaimann
added a commit
to chaimann/solidus
that referenced
this pull request
Jun 17, 2025
tvdeyen
force-pushed
the
admin-flash-helper
branch
from
85ab367 to
4f313e5
Compare
tvdeyen
approved these changes
Jun 27, 2025
There was a problem hiding this comment.
I am not very happy with mis-using the Rails flash hash like this, but I this is a very pragmatic and useful way of implementing the UI guidelines (which make sense).
I have a non-block suggesting for making the naming a bit more neutral. We should/could amend tests to the commits introducing the tested code.
| attr_reader :alerts | ||
|
|
||
| # Construct alert flashes like: | ||
| # flash[:alert] = { <alert_type>: { title: "", description: "" } } |
There was a problem hiding this comment.
description seems to be non-intuitive, since the text is not always necessarily descriptive. What about more neutral message?
chaimann
force-pushed
the
admin-flash-helper
branch
from
4f313e5 to
d0a7973
Compare
4 tasks
Since we now have a dedicated UI component for Alerts, we should be able to distinguish which flash type goes where (toast or alert). This helper adds two methods: #toasts will collect any flash key other than `:alert`, ideally we use "notice" and "error" but it's not forced; #alerts will return everything in `flash[:alert]`; `:alert` flash key should be used to construct flashes that are displayed by Alert component.
`flash[:notice]` is what is commonly used in other controllers to display a regular success/notice toast, so to be consistent let's use it everywhere. `:success` would also work, but to avoid confusion with Alerts of "success" type (see "ui/alert" component) it's better to use correct naming everywhere.
This should allow to target them with turbo stream responses when needed
Passing strings as messages to `flash[]` is more common than using other primitives, and users deciding to set flash[:alert] might not be aware of the Hash structure that is required to correctly construct and display a UI alert from flash[:alert]. So instead of letting it fail because of the type mismatch (when calling #slice on a string instead of a hash), we can add a fallback option to treat given string as a body of the alert message and construct a `danger` type alert with default title.
tvdeyen
force-pushed
the
admin-flash-helper
branch
from
d0a7973 to
27adf3d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
Split from #6228 and #6236
Summary
Since we now have a dedicated UI component for Alerts, we should be able to distinguish which flash type goes where (toast or alert). FlashHelper adds two methods:
#toastswill collect any flash key other than:alert, ideally we use "notice" and "error" but it's not forced;#alertswill return everything inflash[:alert].:alertflash key should be used to construct flashes that are displayed by Alert component. It requires a Hash structure of sortflash[:alert] = { <alert_type>: { title: String, description: String } }to be displayed correctly, but if user passes just a string e.g.flash[:alert] = "Things went horribly wrong"we take the string and create a structure to pass to Alert component:{ danger: { description: "Things went horribly wrong" } }, so that the user is not left with an exception saying that#slicemethod received an incorrect number of arguments to figure it out on their own.Updates Alert component to account for default title to be used in such cases.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: