- 🇺🇸United States smustgrave
This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.
For the new configuration
+ thousand_separator: + type: text + label: 'Thousand marker'
Will need a update hook to update views using that block. So users aren't surprised later if they just save the view.
Will need a test for the update hook too.Everything else looked good.
Thanks!
- First commit to issue fork.
- Merge request !8613Draft: Issue #3034161 by palmyr, Pancho, alireza.tayari, ranjith_kumar_k_u, jeni_dc,... → (Open) created by jrglasgow
- 🇺🇸United States jrglasgow Idaho
the patch from #20 wasn't applying to D10.3, I manually applied and created a fork for the issue.
- First commit to issue fork.
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇮🇳India mrinalini9 New Delhi
Fixed bot issues in patch #34 as mentioned in #35, please review it.
Thanks!
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇯🇵Japan neptune-dc
I applied the MR and confirmed that it correctly worked with any separator, such as ` ` or `,` or `.` .
It seems a little strange that we are adding a separator for the footer but not the pager itself, but I suppose that is out of scope.
- 🇺🇸United States xjm
Thanks everyone for working on this issue!
The first question I ask myself when I review an issue is, "Should this change even be made in the first place?" For core, we additionally ask, "Does this belong in core?"
Unfortunately, every additional element added to the user interface actually decreases usability. During the Views in Drupal Core Initiative, we went through a massive effort with the Usability topic maintainers at the time to remove as many form elements and descriptions from the user interface as possible. So, based on that alone, I'm hesitant to add any small configuration elements to the core Views schemata. Views is, after all, highly extensible, so elements can also be added in contrib.
Furthermore, the separator used in numbers is actually dependent on the language used, so configuring this independently of the site language, config translation, and user's individual translation is somewhat against pattern.
As a Views subsystem maintainer, I'm going to say this feature won't be added to Views in core, and can instead be implemented in contrib.
Since folks have put so much time into this issue over the years, I'm saving the issue credits according to the core issue credit guidelines → . ( Wontfixed issues receive credit now too ✨ Grant credit for all closed issues, not just fixed issues Active .)
Thanks everyone for your contributions!
- 🇺🇸United States xjm
I don't think that's correct actually -- 11.2.x is the current production branch. This change would never be backported to a patch release.