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

Use generic labels in DCAs #532

Merged
merged 6 commits into from Jun 25, 2019

Conversation

@leofeyer
Copy link
Member

commented Jun 24, 2019

This PR implements the changes discussed in #509.

  1. It is no loner necessary to add the label key in the DCA for (global) operations and fields. It will be set automatically if not given.

  2. It is possible to define a special _element label per table, which will be used to replace the word "element" with the label content, so e.g. "edit element ID 2" becomes "edit user group ID 2".

  3. The alt tags of the IMG elements are now the same as the title tags of the A elements, which fixes the accessibility of the buttons (see https://ux.stackexchange.com/a/92259).

@contao/developers Let me know if you like the concept, then I will complete the PR.

TODO

  • XLIFF files
  • Other bundles

@leofeyer leofeyer added the feature label Jun 24, 2019

@leofeyer leofeyer added this to the 4.8 milestone Jun 24, 2019

@leofeyer leofeyer requested a review from ausi Jun 24, 2019

@leofeyer leofeyer self-assigned this Jun 24, 2019

@ausi

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Will the _element label work for every language? Do we need a plural _elements too?

@Toflar

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

In other languages there‘s not just singular and plural afaik. Or maybe there is but the rules are different. Maybe we can use doctrine/inflector? Or Symfony (not sure if English only).

@leofeyer

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2019

Will the _element label work for every language? Do we need a plural _elements too?

I am only using the _element label for singular nouns ("edit article", "show article settings", "delete article", etc.), so it should be fine. There have not been any plural nouns so far.

Maybe we can use doctrine/inflector?

We are not inflecting anything. It is a simple replacement of the word "element" with whatever the content of $GLOBALS['TL_LANG'][$table]['_element'] is.

@aschempp
Copy link
Contributor

left a comment

The DcaLoader does currently not load the language files (DC_Table does), so the translations wont work in all cases.

@leofeyer

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2019

Unfortunately, _element does not work because of the definite article in German ("Details des Elements" vs "Details der Seite"). So I am going to revert this.

Nevertheless, I am not really happy with using "element" everywhere, because it decreases the accessibility IMHO. It is a lot clearer if a button says "delete member group ID %s" than if it says "delete element ID %s".

@leofeyer leofeyer force-pushed the feature/labels branch from e85dfdf to 946dfe8 Jun 24, 2019

@leofeyer leofeyer force-pushed the feature/labels branch from ebb4222 to 96ecc84 Jun 24, 2019

@leofeyer leofeyer marked this pull request as ready for review Jun 25, 2019

@leofeyer

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2019

The PR is now ready to merge. Even though I decided against using "element" everywhere (see above), it was possible to remove a lot of lines in DCA and language files. Plus we are fixing the alt attributes of the buttons, so this is still an improvement.

@Toflar

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

Plus we‘re making it easier for third party bundles. So this is actually a huge improvement :)

@ausi
ausi approved these changes Jun 25, 2019

@leofeyer leofeyer merged commit 07bb454 into master Jun 25, 2019

5 checks passed

Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage remained the same at 86.956%
Details

@leofeyer leofeyer deleted the feature/labels branch Jun 25, 2019

@@ -447,8 +448,7 @@ public function showAll()
$return = $this->panel() . Message::generate() . '
<div id="tl_buttons">'.((Input::get('act') == 'select') ? '
<a href="'.$this->getReferer(true).'" class="header_back" title="'.StringUtil::specialchars($GLOBALS['TL_LANG']['MSC']['backBTTitle']).'" accesskey="b" onclick="Backend.getScrollOffset()">'.$GLOBALS['TL_LANG']['MSC']['backBT'].'</a> ' : '') . ((Input::get('act') != 'select' && !$blnClipboard && !$GLOBALS['TL_DCA'][$this->strTable]['config']['closed'] && !$GLOBALS['TL_DCA'][$this->strTable]['config']['notCreatable']) ? '
<a href="'.$this->addToUrl($hrfNew).'" class="'.$clsNew.'" title="'.StringUtil::specialchars($ttlNew).'" accesskey="n" onclick="Backend.getScrollOffset()">'.$lblNew.'</a>
<a href="'.$this->addToUrl('&amp;act=paste&amp;mode=move').'" class="header_new" title="'.StringUtil::specialchars($GLOBALS['TL_LANG'][$this->strTable]['move'][1]).'" onclick="Backend.getScrollOffset()">'.$GLOBALS['TL_LANG'][$this->strTable]['move'][0].'</a> ' : '') . ($blnClipboard ? '

This comment has been minimized.

Copy link
@aschempp

aschempp Jun 25, 2019

Contributor

why did you remove this operation?

This comment has been minimized.

Copy link
@leofeyer

leofeyer Jun 25, 2019

Author Member

It is no longer required. There are "upload files" link in each folder or you can upload files with drag and drop.

This comment has been minimized.

Copy link
@aschempp

aschempp Jun 25, 2019

Contributor

uhm, so we also don't need the New button at the top of content elements (because there's a new button on each row)?

This comment has been minimized.

Copy link
@leofeyer

leofeyer Jun 26, 2019

Author Member

You cannot create new content elements by dragging them, can you? So yes. (BTW, there is still a "New" button in the file manager to create new folders.)

@@ -4628,6 +4647,9 @@ protected function listView()
$firstOrderBy = $this->firstOrderBy;
}
// Check the default labels (see #509)
$labelNew = $GLOBALS['TL_LANG'][$this->strTable]['new'] ?? $GLOBALS['TL_LANG']['DCA']['new'];

This comment has been minimized.

Copy link
@aschempp

aschempp Jun 25, 2019

Contributor

If you gast this to an array, then the value would become the button if there is only one. No title then, but I guess thats fine?

@@ -639,75 +639,48 @@
<source>Hide the author</source>
</trans-unit>
<trans-unit id="tl_content.new.0">
<source>New element</source>

This comment has been minimized.

Copy link
@aschempp

aschempp Jun 25, 2019

Contributor

Can we not keep this as it was?

<trans-unit id="tl_content.pasteafter.0">
<source>Paste at the top</source>
</trans-unit>
<trans-unit id="tl_content.pasteafter.1">
<source>Paste after content element ID %s</source>
</trans-unit>
<trans-unit id="tl_content.pastenew.0">
<source>Add new at the top</source>

This comment has been minimized.

Copy link
@aschempp

aschempp Jun 25, 2019

Contributor

Changing labels that don't bring much benefit requires translators to translate everything all over again…

</trans-unit>
<trans-unit id="MSC.all_override.0">
<source>Override multiple</source>
</trans-unit>
<trans-unit id="MSC.all_override.1">
<source>Override multiple records at once</source>
<source>Override multiple elements at once</source>

This comment has been minimized.

Copy link
@aschempp

aschempp Jun 25, 2019

Contributor

Are you intentionally change this? Just asking because you reverted the "element" stuff. "Elements" to me is for content elements, not database records in general.

This comment has been minimized.

Copy link
@leofeyer

leofeyer Jun 26, 2019

Author Member

We have agreed on using "elements" by default.

<trans-unit id="DCA.new.0">
<source>New</source>
</trans-unit>
<trans-unit id="DCA.new.1">
<source>Create a new element</source>
</trans-unit>
<trans-unit id="DCA.show">
<source>Show the details of element ID %s</source>
</trans-unit>
<trans-unit id="DCA.edit">
<source>Edit element ID %s</source>
</trans-unit>
<trans-unit id="DCA.cut">
<source>Move element ID %s</source>
</trans-unit>
<trans-unit id="DCA.copy">
<source>Duplicate element ID %s</source>
</trans-unit>
<trans-unit id="DCA.delete">
<source>Delete element ID %s</source>
</trans-unit>
<trans-unit id="DCA.editheader.0">
<source>Edit the element settings</source>
</trans-unit>
<trans-unit id="DCA.editheader.1">
<source>Edit the settings of element ID %s</source>
</trans-unit>
<trans-unit id="DCA.toggle">
<source>Publish/unpublish element ID %s</source>
</trans-unit>
<trans-unit id="DCA.pasteinto.0">
<source>Paste at the top</source>
</trans-unit>
<trans-unit id="DCA.pasteinto.1">
<source>Paste into element ID %s</source>
</trans-unit>
<trans-unit id="DCA.pasteafter.0">
<source>Paste at the top</source>
</trans-unit>
<trans-unit id="DCA.pasteafter.1">
<source>Paste after element ID %s</source>
</trans-unit>
<trans-unit id="DCA.pastenew.0">
<source>Create a new element at the top</source>
</trans-unit>
<trans-unit id="DCA.pastenew.1">
<source>Create a new element after element ID %s</source>
</trans-unit>

@BugBuster1701

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2019

Wenn die Übersetzungen fehlen für die generischen Labels und der Cache wird ungünstig aufgebaut, dann hat das ganz seltsame Auswirkungen. Siehe Slack.
https://contao.slack.com/archives/CK4J0KNDB/p1563661670012700

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.