-
Notifications
You must be signed in to change notification settings - Fork 22
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
Fix empty shortcode output #95
base: trunk
Are you sure you want to change the base?
Conversation
@pkevan Since you dug into the original report a bit, would you be able to review this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to be careful with outputting messages to end users here (maybe an error warning which doesn't show in the front-end would be better).
@pkevan, in any case, in my opinion, it is necessary to make some kind of display of an explanation of why the shortcode "does not work". In my experience, this raises questions and bewilderment among some users. |
What about having the message only displayed for logged-in admins? |
After my last post I thought the same thing - display for admin, something like that:
|
Also, my bad, I think it would be safer to organize the output as follows:
|
That could work - or even have that cap check filtered? As it might be better to have editors/admins included by default (on the basis on advanced privileges) Given there are multiple instances of outputting based on which settings are set outside of the shortcode, we should probably cover all of those which return an empty response. |
Fixies Issue #85 (Shortcode Bug)
Perhaps the text of the message needs to be changed as I am not a native speaker.