OG UI: UX improvement on OG roles and permissions overview config pages [#1794950]

Comment	File	Size	Author
#8	og-ux-new-link-1794950-8.patch	544 bytes	alesr

#3	og-ux-new-link-1794950-3.patch	553 bytes	alesr

#1	og_after.png	10.52 KB	alesr
#1	og-ux-new-link-1794950-1.patch	542 bytes	alesr

	og_before.png	10.11 KB	alesr

Comment #1

alesr CreditAttribution: alesr commented 25 September 2012 at 21:41

Status:

Active

» Needs review

File	Size
og-ux-new-link-1794950-1.patch	542 bytes

og_after.png	10.52 KB

A picture is worth a thousand words.
Compare og_before.png and og_after.png

Image and patch are attached.

Log in or register to post comments

Comment #2

amitaibu

Hebrew

Israel

CreditAttribution: amitaibu commented 26 September 2012 at 14:02

Status:

Needs review

» Needs work

+++ b/og_ui/og_ui.admin.incundefined
@@ -260,7 +260,7 @@ function og_ui_group_types_overview($type) {
+    '#empty' => t('No group types available. Edit or !add_new and set it to behave as a group.', array('!add_new' => l(t('add a new content type'), 'admin/structure/types/add'))),

Instead of !add new, you can use <a href="@url">add a new content...

Like this the message can be translated as once sentence.

Log in or register to post comments

Comment #3

alesr CreditAttribution: alesr commented 26 September 2012 at 14:43

Status:

Needs work

» Needs review

File	Size
og-ux-new-link-1794950-3.patch	553 bytes

Thanks for review Amitaibu.
I made a new patch that is translator friendlier.

Log in or register to post comments

Comment #4

amitaibu

Hebrew

Israel

CreditAttribution: amitaibu commented 26 September 2012 at 18:19

Status:

Needs review

» Needs work

+++ b/og_ui/og_ui.admin.incundefined
@@ -260,7 +260,7 @@ function og_ui_group_types_overview($type) {
+    '#empty' => t('No group types available. Edit or <a href="@url">add a new content type</a> and set it to behave as a group.', array('@url' => base_path() . 'admin/structure/types/add')),

Don't use base_path(), always use l()

Log in or register to post comments

Comment #5

alesr CreditAttribution: alesr commented 26 September 2012 at 19:08

Are you sure about l()? Because after your suggestion from #2 I changed the link to <a href="@url">... so the translators would be happy.
If I use it in array('@url' =>l('text', 'path')), <a href> inside <a href> is generated.
url() is I guess a better solution in this case or am I wrong?

Do you agree with the part of code below so I can make the final patch?
array('@url' => url('admin/structure/types/add'))