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

[4.0] Moving reusable parts of blankstate layout to a JLayout #33288

Merged
merged 14 commits into from
Apr 25, 2021

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Apr 24, 2021

Pull Request for Issue #33279 .

Summary of Changes

  • Moved the repeating parts of the layout to a JLayout which can be used also by 3rd parties.
  • Added permissions checks for the "Add xxx" button. The permissions checks are kept as similar as reasonable to the one for the toolbar button
  • Corrected the link of the "Add xxx" button to point to the respective controller task instead of directly going to the view.
  • Adjusted the language strings for the "_CONTENT" so they only differate in the prefix. This could also be changed to "_ITEMS" but I think basically everything is "content" of some sort as well, not only com_content items. In the end, the name of the string doesn't matter anyway much.
  • Move the "Learn More" string to the joomla.ini file, because this one always is the same.

Testing Instructions

Check the newly introduced blankstate layouts. They should look the same before and after this PR.

Documentation Changes Required

Should be included in the documentation about blankstate layouts when/if/once that exists

@Bakual
Copy link
Contributor Author

Bakual commented Apr 24, 2021

One thing I wondered: The "Learn More" link currently opens in the same tab. Maybe it makes more sense to open it in a new tab? Since I guess usually the people going to read the help page don't actually want to leave the admin backend.

@PhilETaylor

This comment was marked as abuse.

@brianteeman
Copy link
Contributor

From an accessibility perspective it is OK to open a new window in this case because the content is designed to be read with the web page. This is even an example use case in the WCAG docs

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@Bakual
Copy link
Contributor Author

Bakual commented Apr 24, 2021

I've added target="_blank" to the help link so it opens in a new tab.

@Bakual
Copy link
Contributor Author

Bakual commented Apr 24, 2021

Please can you check #33286 which was done at the same time as your great work to see if either of us needs to change much (other than the normal complete refactoring) due to the addition of the extension part.

I think from a first look of it, it should work fine with this PR

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@Bakual
Copy link
Contributor Author

Bakual commented Apr 24, 2021

adding target=_blank gives this horrid icon, its not even inline pixel perfectly...

Didn't even notice this, but you're right. It doesn't look nice. Open for suggestions (could also be done in a separate PR).

@PhilETaylor

This comment was marked as abuse.

Co-authored-by: Brian Teeman <brian@teeman.net>
@PhilETaylor

This comment was marked as abuse.

@joomdonation
Copy link
Contributor

I have tested this item ✅ successfully on 17298b6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33288.

@alikon
Copy link
Contributor

alikon commented Apr 25, 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33288.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 25, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone Apr 25, 2021
@richard67 richard67 merged commit 128897a into joomla:4.0-dev Apr 25, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 25, 2021
@richard67
Copy link
Member

Thanks!

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants