Create configuration schemas for toolbar module [#1919212]

This is a sub-issue of #1910624: [META] Introduce and complete configuration schemas in all of core.

Problem/motivation

#1866610: Introduce Kwalify-inspired schema format for configuration introduced the idea of config schema. The changelog leads to (hopefully extensive) documentation on the format at http://drupal.org/node/1905070. While there are little cleanups planned for the format overall, the current format is a result of months of back and forths, so it should be perfectly fine to apply it more widely to core.

Proposed solution

Create a configuration schema for toolbar module.

Schema in place

Schema not yet in place
toolbar.breakpoints.yml
toolbar.config.yml

Comment	File	Size	Author
#12	toolbar_schema.png	653.09 KB	YesCT
#12	toolbar_configure.png	166.28 KB	YesCT
#8	Selection_045.png	20.32 KB	dsnopek
#8	Selection_046.png	16.46 KB	dsnopek
#8	1919212-toolbar-schema-8.patch	768 bytes	dsnopek
#8
#3	toolbar_scema.PNG	57.57 KB	rahuldolas123
#2	1919212-toolbar-schema-2.patch	725 bytes	vyasamit2007
#2
#1	1919212-toolbar-schema-1.patch	588 bytes	vijaycs85
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 17 February 2013 at 16:27

Status:

Active

» Needs review

File	Size
1919212-toolbar-schema-1.patch	588 bytes

Adding schema file...

Comment #2

vyasamit2007 CreditAttribution: vyasamit2007 commented 23 February 2013 at 11:53

File	Size
1919212-toolbar-schema-2.patch	725 bytes

Schema file with new changes and as per the updated schema documentation http://drupal.org/node/1905070#codestyle

Comment #3

rahuldolas123 CreditAttribution: rahuldolas123 commented 23 February 2013 at 12:24

Status:

Needs work

» Needs review

File	Size
toolbar_scema.PNG	57.57 KB

Hi,

I tested the patch with config inspector module and I can only see the forms for 'toolbar.breakpoints.yml' file.
The form for 'toolbar.config.yml' file is not shown. The file toolbar.config.yml should be renamed to 'toolbar.settings.yml' so that both the settings can be visible.

screenshots:
toolbar patch

toolbar.settings not shown:

Comment #4

rahuldolas123 CreditAttribution: rahuldolas123 commented 23 February 2013 at 12:19

Status:

Needs review

» Needs work

Comment #5

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 23 February 2013 at 12:34

Status:

Needs review

» Postponed

Thanks @rahuldolas123. I have raised an issue #1925380: Rename toolbar.config.yml to toolbar.settings.yml to see if it has some reason to name it that way. For now, we leave schema file as it is. We will change, if it config file has to be toobar.config.yml.

postpone-ding until #1925380: Rename toolbar.config.yml to toolbar.settings.yml get fixed...

Comment #6

aspilicious CreditAttribution: aspilicious commented 3 March 2013 at 14:48

Status:

Postponed

» Active

Comment #7

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 3 March 2013 at 17:04

Status:

Active

» Needs review

Patch in #2 is ready for review now

Comment #8

dsnopek

he/him

English

USA

CreditAttribution: dsnopek commented 8 March 2013 at 02:42

File	Size
1919212-toolbar-schema-8.patch	768 bytes

Selection_046.png	16.46 KB
Selection_045.png	20.32 KB

Looking at the form for toolbar.settings in the config_inspector throws an error:

Notice: Array to string conversion in Drupal\Core\TypedData\TypedData->getString() (line 69 of core/lib/Drupal/Core/TypedData/TypedData.php).

+++ b/core/modules/toolbar/config/schema/toolbar.schema.yml
+    breakpoints:
+      type: string
+      label: 'Breakpoints'

This should really be 'sequence' not 'string'. Attached is a new patch that fixes this!

However, no matter what I set for label it's always 'String' for the individual items of the sequence. Is it not possible to set labels on individual items of a sequence? Or is this a bug in the config_inspector?

Comment #9

sandipmkhairnar CreditAttribution: sandipmkhairnar commented 8 March 2013 at 18:15

I have reviewed and apply the patch . Its working fine for me.

Comment #10

tim-e CreditAttribution: tim-e commented 9 March 2013 at 01:05

Status:

Needs review

» Reviewed & tested by the community

Looking good to me

Comment #11

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 9 March 2013 at 05:30

Status:

Reviewed & tested by the community

» Fixed

Committed and pushed to 8.x. Thanks!

Comment #12

YesCT CreditAttribution: YesCT commented 9 March 2013 at 07:01

Status:

Fixed

» Reviewed & tested by the community

File	Size
toolbar_configure.png	166.28 KB
toolbar_schema.png	653.09 KB

config_inspector was updated, had to get the latest.

+++ b/core/modules/toolbar/config/schema/toolbar.schema.ymlundefined
@@ -0,0 +1,25 @@
+      sequence:
+        - type: string
+

the items in the sequence dont have a Label, so that is why they are all string.

I agree, no errors and everything looks good.

so still rtbc.

question: is there any form in the ui for the toolbar breakpoint settings? (toolbar.breakpoints.yml)
The toolbar configure link does not actually go anywhere.

Comment #13

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 9 March 2013 at 14:25

Status:

Reviewed & tested by the community

» Fixed

Hm. I think that might've been a cross-post?

Comment #14

dsnopek

he/him

English

USA

CreditAttribution: dsnopek commented 9 March 2013 at 16:19

@YesCT: I tried adding a label but it had no effect so, I didn't include it in the patch. Also, I'm not sure what the label should be! Should every item in the sequence have the label "Breakpoint"? Really, they should probably be labeled: 1, 2, 3, ... etc.