Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#43434 closed enhancement (wontfix)

Add sprintf function and target attribute in href

Reported by: mukesh27's profile mukesh27 Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.9.4
Component: Help/About Keywords: 2nd-opinion
Focuses: Cc:

Description

It is better to use sprintf in static link also add target blank attribute in link so user can redirected to proper document of WordPress.

Attachments (1)

code.patch (1.1 KB) - added by mukesh27 7 years ago.
Code patch

Download all attachments as: .zip

Change History (11)

@mukesh27
7 years ago

Code patch

#1 @mukesh27
7 years ago

  • Keywords dev-feedback needs-testing added

#2 @jrf
7 years ago

  • Focuses coding-standards removed

#3 follow-up: @audrasjb
7 years ago

  • Focuses coding-standards added
  • Keywords i18n-change 2nd-opinion added; dev-feedback needs-testing removed

Hello @mukesh27

Thanks for this ticket.
However, in my opinion:

  • Similar links are used in many help panels, like widgets.php for example. Any change on users.php should be consistent in other screens.
  • Target blank is very often a bad idea. At least, it should be identified as "opens-in-a-new-window" links.
  • I do not think using sprintf is relevant as some Locales can replace the links with their own translated resources.

#4 @audrasjb
7 years ago

  • Focuses coding-standards removed

#5 follow-up: @SergeyBiryukov
7 years ago

  • Component changed from General to Users
  • Focuses administration added; accessibility removed
  • Keywords i18n-change removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Hi @mukesh27, welcome to WordPress Trac! Thanks for the patch.

#6 in reply to: ↑ 3 @mukesh27
7 years ago

  • Component changed from Users to General
  • Focuses accessibility added; administration removed
  • Keywords i18n-change added
  • We have to change that link as it's part of coding standard.
  • Yes, right now when user click on that help artical then that page open in current tab so needs to open in new tab so user can see that document and not lost there current page.
  • Yes but we have to add sprintf so all Locales can replace string if they want.

Replying to audrasjb:

Hello @mukesh27

Thanks for this ticket.
However, in my opinion:

  • Similar links are used in many help panels, like widgets.php for example. Any change on users.php should be consistent in other screens.
  • Target blank is very often a bad idea. At least, it should be identified as "opens-in-a-new-window" links.
  • I do not think using sprintf is relevant as some Locales can replace the links with their own translated resources.

#7 in reply to: ↑ 5 @mukesh27
7 years ago

Why you are close this ticket as we have to add sprintf for translation. understood for target

Replying to SergeyBiryukov:

Hi @mukesh27, welcome to WordPress Trac! Thanks for the patch.

#8 @audrasjb
7 years ago

@mukesh27
There is already a __( ) function, which is already used for translation.
No need to sprintf( ) here. See https://developer.wordpress.org/reference/functions/__/

#9 @afercia
7 years ago

@mukesh27 generally, it's a good practice to keep links out from the translatable strings using sprintf() as gou suggested, but as @SergeyBiryukov pointed out, all the links to the Codex pages need to be translatable. Sometimes, the codex pages have versions in different languages. Translators need a way to change the link to point to the one in their language, if they want.
For example:
https://codex.wordpress.org/Users_Screen
https://codex.wordpress.org/zh-cn:%E7%94%A8%E6%88%B7%E9%A1%B5%E9%9D%A2
https://codex.wordpress.org/pt-br:SubPainel_Todos_os_Usu%C3%A1rios

#10 @SergeyBiryukov
7 years ago

  • Component changed from General to Help/About
  • Focuses accessibility removed
  • Keywords i18n-change removed
Note: See TracTickets for help on using tickets.