Comments

loopduplicate created an issue. See original summary.

aditya_anurag’s picture

Assigned: Unassigned » aditya_anurag
aditya_anurag’s picture

Status: Active » Needs review
StatusFileSize
new1.1 KB

Thanks for reporting the issue.
I think text formater is not required here so we can remove it. I created the patch please have a look.

krknth’s picture

Assigned: aditya_anurag » krknth

checking ..

rakesh.gectcr’s picture

Status: Needs review » Reviewed & tested by the community

If we don't need the text format, the patch is fine.

joachim’s picture

Status: Reviewed & tested by the community » Needs work

I'm not sure that removing the format is the right thing to do here.

Also, if you *did* want to remove this functionality, you'd need to also clean up code that uses the setting, such as:

  $output .= '<div>' . check_markup($variables['description'], variable_get('role_help_format', filter_fallback_format()), FALSE) . '</div>';
n.kishorekumar’s picture

Assigned: krknth » n.kishorekumar

code cleanup by removing all unused variable of role_help_format

n.kishorekumar’s picture

Status: Needs work » Needs review
StatusFileSize
new7.77 KB

code cleanup by removing all unused variable of role_help_format

n.kishorekumar’s picture

n.kishorekumar’s picture

Please dont use the previous patch.missed coding standards in that

Use this patch
code cleanup and by removing all unused variable of role_help_format and added interdiff file with this

joachim’s picture

Status: Needs review » Postponed (maintainer needs more info)

> I'm not sure that removing the format is the right thing to do here.

Could someone explain the reasoning for removing this functionality?

n.kishorekumar’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new857 bytes

@joachim : I think #3 was mistaken. I updated the patch which can work with formatter.

imalabya’s picture

@N.kishorekumar
$row[] = t('@name <em>(locked)</em>', array('@name' => $name)) . '<br><div class="description">' . check_markup($summary,variable_get('role_help_format')) . '</div>';

Please Add a default value for the variable_get. Even it is not effecting the functionality it is good practice to have a fallback value when using variable_get.

n.kishorekumar’s picture

@malabya Thanks for reviewing the patch

Added the fallback value in the variable_get

krknth’s picture

Title: Unable to choose the role help text format » Text Format field doesn't apply for roles description
Issue summary: View changes
Status: Needs review » Needs work

Let me clarify why we need text_format form field in the form.

Form text_format using to set the input format for all role help text on the role help settings page.

@loopduplicate : The text_format form field showing up in the admin/config/people/role_help/ page

*I'm changing the title of the issue*

@aditya_anurag: Yes i agree, we might not require text_form field as it is just help text & we can simply show it as plain text. But let keep it as optional might be someone require it for design perspective.

@N.kishorekumar : Please check on help/roles page, its working as expected

But I think we need text format for other roles also (on role edit pages).

@Note : text_format not required for description of roles & it only shows on help/roles page.

krknth’s picture

To avoid to confusion - lets add Summary text field in help/roles page. So it will be clear that the purpose of summary & help text fields.

krknth’s picture

Title: Text Format field doesn't apply for roles description » Unable to choose the role help text format
Issue summary: View changes

Reverting issue metadata to original

krknth’s picture

Status: Needs work » Closed (works as designed)

Better to follow this issue #2686441: Standardize Summary, Help text fields for anonymous and authenticated roles..

I'm closing this issue because as per issue description it was working as designed.