Add a @see + docs about language_element_info_alter() to LanguageSelect [#2505197]

Problem/Motivation

\Drupal\Core\Render\Element\LanguageSelect is a form element that - without rendering an actual form element - always returns the value of the default language. It is then extended by Language module via language_element_info_alter() to provide a proper language selector.

This is done so that modules can provide language integration without having to depend on Language module or introducing a $module_handler->moduleExists('language') check and Language module doesn't have to introduce knowledge about any particular module. In short: It's a good thing! :-)

However, when just looking at the LanguageSelect class this is not very obvious, because there is no documentation about this fact. The @see to \Drupal\Core\Render\Element\Select doesn't help either (and arguably makes things more confusing).

Proposed resolution

1. Add documentation to the class about the above.
2. Add a @see to language_element_info_alter()

3. Optionally consider moving the @see to \Drupal\Core\Render\Element\Select over to language_element_info_alter().

Remaining tasks

Write patch! :-)

User interface changes

None.

API changes

None.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category	Task because the added documentation improves DX but nothing is broken/incorrect about the current docs (it's just lacking).
Issue priority	Normal
Unfrozen changes	Unfrozen because it only changes documentation

Comment	File	Size	Author
#18	adds-documentation-to-languageselect-2505197-18.patch	1.23 KB	arskiainen
#18
#16	adds-documentation-to-languageselect-2505197-16.patch	1.23 KB	arskiainen
#16
#12	adds-documentation-to-languageselect-2505197-12.patch	1.23 KB	jaimeguzman
#12
#8	adds-documentation-to-languageselect-2505197-8.patch	1.22 KB	arskiainen
#8
#5	adds-documentation-to-languageselect-2505197-5.patch	1.25 KB	arskiainen
#5
#2	adds-documentation-to-languageselect-2505197-2.patch	1.38 KB	arskiainen
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 12 June 2015 at 13:13

Issue summary:

View changes

Comment #2

arskiainen CreditAttribution: arskiainen as a volunteer commented 13 June 2015 at 11:11

File	Size
adds-documentation-to-languageselect-2505197-2.patch	1.38 KB

If I understood the issue correctly, this should provide resolution.

Comment #3

arskiainen CreditAttribution: arskiainen as a volunteer commented 13 June 2015 at 11:23

Assigned:	Unassigned	» arskiainen
Status:	Active	» Needs review

Comment #4

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 13 June 2015 at 12:31

Awesome, thanks @Arskiainen. This is exactly what I meant.

Unfortunately, I cannot mark the patch RTBC (yet!) because our documentation standards require each class documentation to start with a one-line summary. So I think it would be preferred to leave the existing one-line summary and change the sentence you added so that it makes sense in the combination (sorry if I'm not making sense, this is hard to explain with words).

Or if you find some other way to convey the information, absolutely feel free to go crazy :-P. It just needs to start with a one-line summary.

Comment #5

arskiainen CreditAttribution: arskiainen as a volunteer commented 13 June 2015 at 12:45

File	Size
adds-documentation-to-languageselect-2505197-5.patch	1.25 KB

Is this what you meant?

Comment #6

13 June 2015 at 12:47

Status:

Needs review

» Needs work

The last submitted patch, 5: adds-documentation-to-languageselect-2505197-5.patch, failed testing.

Comment #7

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 13 June 2015 at 12:54

Yes, except that now both sentences start with

Provides a Form element

which sounds a bit strange in my opinion. So I think we need to rephrase this somehow.

(Sorry for being so pedantic :-/)

Comment #8

arskiainen CreditAttribution: arskiainen as a volunteer commented 13 June 2015 at 12:59

File	Size
adds-documentation-to-languageselect-2505197-8.patch	1.22 KB

How about now?

Comment #9

arskiainen CreditAttribution: arskiainen as a volunteer commented 13 June 2015 at 13:01

Status:

Needs work

» Needs review

Comment #10

13 June 2015 at 13:03

Status:

Needs review

» Needs work

The last submitted patch, 8: adds-documentation-to-languageselect-2505197-8.patch, failed testing.

Comment #11

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 13 June 2015 at 14:14

Yes, that sounds very good!

I found two more super minor things, sorry. I promise I'll RTBC the next patch :-)

1. The sentence is now actually missing a subject, so I would suggest to add a "This" in the beginning of the sentence.

2. We are not allowed to use contractions in Drupal core, so "Doesn't" should be "does not". Sorry for just pointing this out now, I should have seen this earlier.

So in summary:
"Doesn't" -> "This does not"
But make sure to break the line at 80 characters.

Thanks for sticking with this, much appreciated!

Comment #12

jaimeguzman CreditAttribution: jaimeguzman as a volunteer commented 13 June 2015 at 18:38

File	Size
adds-documentation-to-languageselect-2505197-12.patch	1.23 KB

HI how about this.

Comment #13

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 13 June 2015 at 18:47

Status:

Needs work

» Reviewed & tested by the community

Yay, thanks @jaimeguzman. Nothing to complain anymore. :-)

@jaimeguzman this issue was still assigned to @Arskiainen which probably means @Arskiainen wanted to finish this issue. That's why it's best to check the "Assigned" field before working on an issue and if you start working on something assign the issue to yourself.

Nevertheless, RTBC.

Comment #14

jaimeguzman CreditAttribution: jaimeguzman as a volunteer commented 13 June 2015 at 18:55

thanks @tstoeckler , was my first patch for help =). I'll check the next time. assigned field.
My apologies @Arskiainen

Comment #15

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 13 June 2015 at 20:04

Status:

Reviewed & tested by the community

» Needs work

Oh no, I found something else. I was without Dreditor earlier, so I missed it:

	diff --git a/core/lib/Drupal/Core/Render/Element/LanguageSelect.php b/core/lib/Drupal/Core/Render/Element/LanguageSelect.php
index 3b01ba1..3b9c2ef 100644
--- a/core/lib/Drupal/Core/Render/Element/LanguageSelect.php
+++ b/core/lib/Drupal/Core/Render/Element/LanguageSelect.php
@@ -12,7 +12,11 @@
+ * This does not render an actual form element, but always returns 
+ * the value of the default language. It is then extended by Language module

There is a trailing whitespace after the "returns" that should be removed. Also, I think the line is breaking to early. the "the value of" should still fit into the next line.

@Arskiainen maybe you can finish this one, then?

Comment #16

arskiainen CreditAttribution: arskiainen as a volunteer commented 15 June 2015 at 06:05

Assigned:	arskiainen	» Unassigned
Status:	Needs work	» Needs review

File	Size
adds-documentation-to-languageselect-2505197-16.patch	1.23 KB

A little clean up.

Comment #17

15 June 2015 at 06:30

Status:

Needs review

» Needs work

The last submitted patch, 16: adds-documentation-to-languageselect-2505197-16.patch, failed testing.

Comment #18

arskiainen CreditAttribution: arskiainen as a volunteer commented 15 June 2015 at 06:55

Status:

Needs work

» Needs review

File	Size
adds-documentation-to-languageselect-2505197-18.patch	1.23 KB

...

Comment #19

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 15 June 2015 at 07:40

Status:

Needs review

» Reviewed & tested by the community

Yay, thanks @Arskiainen!

Let's get this in.

docs++
confusion--

Comment #20

jaimeguzman CreditAttribution: jaimeguzman as a volunteer commented 15 June 2015 at 14:05

Great @Arskiainen

Cheers guys !

Comment #21

17 June 2015 at 09:12

Status:

Reviewed & tested by the community

» Needs work

The last submitted patch, 18: adds-documentation-to-languageselect-2505197-18.patch, failed testing.

Comment #22

17 June 2015 at 09:14

Status:

Needs work

» Needs review

tstoeckler queued 18: adds-documentation-to-languageselect-2505197-18.patch for re-testing.

Comment #23

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 17 June 2015 at 09:14

Status:

Needs review

» Reviewed & tested by the community

FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].

Comment #24

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Chapter Three commented 23 June 2015 at 17:31

Status:

Reviewed & tested by the community

» Fixed

Documentation improvements are permitted during beta. Committed ac8605b and pushed to 8.0.x. Thanks!

Comment #25

23 June 2015 at 17:31

alexpott committed ac8605b on 8.0.x

Issue #2505197 by Arskiainen, jaimeguzman, tstoeckler: Add a @see + docs...

Comment #26

7 July 2015 at 17:34

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Comment #27

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy at Acquia commented 16 July 2015 at 14:12

Issue tags:

-sprint

Superb, thanks!