Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Few labels missing in system.site schema that found with the D8 configuration inspector sandbox module at http://drupal.org/sandbox/reyero/1635230
Comment | File | Size | Author |
---|---|---|---|
#25 | 1922270-system-site-schema-labels-25.patch | 1.77 KB | vijaycs85 |
#25 | 1922270-diff-22-25.txt | 980 bytes | vijaycs85 |
#24 | no-period-maintenance.png | 200.23 KB | YesCT |
#22 | 1922270-system-site-schema-labels-22.patch | 1.79 KB | vijaycs85 |
#22 | 1922270-diff-18-22.txt | 1.35 KB | vijaycs85 |
Comments
Comment #1
vijaycs85Adding patch and screenshot before and after fix
Before patch:
After patch:
Comment #2
Gábor HojtsySuperb, let's add a label for the mapping too :) Otherwise looks good.
Comment #3
vijaycs85Thanks @Gábor Hojtsy for the quick review. Updating patch and after fix screenshot.
Comment #4
vijaycs85Please ignore patch in #3. It has code for some other fix. Adding right patch.
Comment #5
Gábor HojtsyLooks good. Completes missing system.site labels.
Comment #6
Gábor HojtsyTagging for config system too.
Comment #7
Gábor HojtsyNeeds a reroll given that #1914366: Move all configuration schema files into a schema subdirectory landed.
Comment #8
vijaycs85Re-rolling with schema file location change
Comment #9
vijaycs85Comment #10
Gábor HojtsyStill good :)
Comment #11
vijaycs85Code style fixes... Ref: http://drupal.org/node/1905070#codestyle
Comment #12
Gábor HojtsyMissing newline between top level file comment and first section.
Comment #13
vijaycs85Updating space...
Comment #14
YesCT CreditAttribution: YesCT commentedshall we add a mention of wanting a newline after the top level comment to http://drupal.org/node/1905070#codestyle ?
Comment #15
YesCT CreditAttribution: YesCT commentedReviewing.
Todo the review, I'm following
http://drupal.org/node/1910624#comment-7088154
and
http://drupal.org/node/1910624#steps-to-test
along with
http://drupal.org/node/1905070#codestyle
side by side compare the data and schema .yml files
I did a side by side comparison manually, visually comparing the schema file and the two data files that the schema describes.
there are actually a lot of data .yml files in system, but the schema lists two: system.site and system.maintenance
(they looked fine)
Aside
Then, I thought I'd look at the form, but the form in the ui looks different than the schema says. Why is that?
back to review
there is a very small nit, to add the word "the" to the file comment. I'll reroll the patch for that.
In the mean time, I'm going to try http://drupal.org/project/config_inspector
Comment #16
YesCT CreditAttribution: YesCT commentedI cannot remember what happened with the discussion of the different types:
label and text. site name is type: label, slogan is type: text.
Comment #17
YesCT CreditAttribution: YesCT commentedlooking in http://drupal.org/node/1905070#types
it has a section:
and defines the types: label, path, and text
Comment #18
YesCT CreditAttribution: YesCT commentedI got the inspector, and checked it there. Things look good and the form looks like @vijaycs85 saw in #3
My other questions here, don't need to hold up this patch, I was just writing them down and they can get answered later.
rtbc from me, but since I changed the code (the comment to match http://drupal.org/node/1905070#codestyle),
I'll wait for someone else to check that change and really rtbc it.
Comment #19
Gábor HojtsyGood find that the texts in the schema do not match the texts on the UI. Eg. "Front page path" vs. "Default front page" where the error paths were well labeled. Also some mismatch with the main item labels (name, slogan, email). It is suggested that the regular form labels are used for consistency.
Comment #20
YesCT CreditAttribution: YesCT commentedWhere do the texts in the UI come from, if not from the system.site.yml or the schema ?
The grouping (mapping level) is a little different in the UI compared to the schema. But the schema and the system.site.yml grouping and mapping levels agree. That is puzzling.
Comment #21
Gábor HojtsyYeah Drupal core will not use these files to generate forms, so comparing Drupal form structure to the schema file based forms is futile. As a general guideline however, it is best to reuse the Drupal form label text for schema item labels, so when contrib generates those translation forms, it looks as close as possible. Take views, we can generate a form for a view in contrib on one screen, but Views will obviously not expose one single form for editing a view.
Comment #22
vijaycs85Updates:
1. replaced type label with string - IMO, we don't need this type as we are overriding label anyway.
2. Labels of fields from configuration page.
3. While searching label for compact admin_compact_mode, found a documentation issue #1928304: Update documentation of admin_compact_mode in system module
4. Adding dot "." for boolean labels.
Comment #23
vijaycs85Comment #24
YesCT CreditAttribution: YesCT commentedwhy add the period there?
Since it's not in the UI, I say leave it out.
Comment #25
vijaycs85Updating as per #24
Comment #27
vijaycs85#25: 1922270-system-site-schema-labels-25.patch queued for re-testing.
Comment #28
Gábor HojtsyLooks good, only found these two issues:
We capitalize module names elsewhere, no? :)
Site slogan is a single line text field. The string type should be used for internal strings like machine names, etc. Exposed textual settings should either use the label type (one line string) or the text type (multiline). The slogan is an exposed free textual setting using a one line input, so it should use type label.
Comment #29
Gábor HojtsyMarked as duplicate of #1912250: Complete configuration schemas for system module, the System module capitalization and the slogan type is correct there. I did not cross-check other changes, please do :)