A suggested configuration is as follows:

I have a few concerns with the configuration.

1. Too expressive.

Configuration that changes appearance should not be added lightly as it increases the amount of variants we need to test on. The feature options seems to go beyond what we need here - for example I would be concerned about allowing this level of granularity per page.

The current system allows us to modify Special:RecentChanges to use large font size, Special:Preferences to use regular and Special:Watchlist to use small. I don't think we need that - it seems from requirements we need just standard (default) or small (exceptions)
An alternative might be something like wgFontSizeSmall which explicitly expresses pages which receive small font size instead of default, however I'd still question whether we need it to be as expressive as limited width - perhaps a list of pages as a first pass might be sufficient for example?

2. Conflict resolution It's not clear to me how we would deal with clashes with this proposed spec - with the current model I can set both 1.for.mainpage and 2.for.mainpage to true and it's not clear what I'd end up with.

3. Coupling with feature management system

Coupling with feature management (e.g. exclude) will likely complicate the feature management further. Since the feature management system only determines whether the feature is enabled or not, I wonder if this could be contained separately (e.g. when enabled what do the defaults look like?

In T363834#9758450, @Jdlrobson wrote:

A suggested configuration is as follows:

I have a few concerns with the configuration.

1. Too expressive.

Configuration that changes appearance should not be added lightly as it increases the amount of variants we need to test on. The feature options seems to go beyond what we need here - for example I would be concerned about allowing this level of granularity per page.

The current system allows us to modify Special:RecentChanges to use large font size, Special:Preferences to use regular and Special:Watchlist to use small. I don't think we need that - it seems from requirements we need just standard (default) or small (exceptions)
An alternative might be something like wgFontSizeSmall which explicitly expresses pages which receive small font size instead of default, however I'd still question whether we need it to be as expressive as limited width - perhaps a list of pages as a first pass might be sufficient for example?

2. Conflict resolution It's not clear to me how we would deal with clashes with this proposed spec - with the current model I can set both 1.for.mainpage and 2.for.mainpage to true and it's not clear what I'd end up with.

3. Coupling with feature management system

Coupling with feature management (e.g. exclude) will likely complicate the feature management further. Since the feature management system only determines whether the feature is enabled or not, I wonder if this could be contained separately (e.g. when enabled what do the defaults look like?

I agree I was trying to show a quick sample that came to my mind while writing the ticket.

I guess the 1st item of the requirement is suggesting and getting a discussion and agreement on a proposed configuration scheme.

the part that was removed from task description based on this discussion is:

A suggested configuration is as follows:

"VectorFeatureOptions": {
  "value": {
    "exclude": {
      "mainpage": false,
      "pagetitles": [],
      "namespaces": [],
      "querystring": {}
    },
    "include": [],
    "defaults": {
      "1": {
        "for": {
          "mainpage": true,
          "pagetitles": [],
          "namespaces": [],
          "querystring": {
            "action": "edit"
          }
        },
        "except": []
      },
      "2": {
        "for": {
          "mainpage": false,
          "pagetitles": [
            "Special:Preferences"
          ],
          "namespaces": [
            12
          ],
          "querystring": {
            "action": "history"
          }
        },
        "except": [
          "Article_Whatever"
        ]
      },
      "3": {
        "for": {
          "mainpage": false,
          "pagetitles": [],
          "namespaces": [
            14
          ],
          "querystring": {
            "action": "view"
          }
        },
        "except": [
          "Special:Search"
        ]
      }
    },
    "description": "Configuration settings to determine feature compatibility and default values based on page type, namespace, or query string. These settings allow for dynamic customisation of user experience."
  }
}

NOTE: The values in the configuration example above are random and do not correlate to our projects, it is just a sample to refine and think about the output configuration in the skin.json file

@ovasileva @JScherer-WMF is this for just desktop or mobile and desktop? (please see this and other open questions and remove and move r to "acceptance criteria")

I was looking into suggesting a newer configuration suggestion, especially adhering to feedback from @Jdlrobson :

"VectorFeatureOptions": {
  "value": {
    "system": {
      "changeValueAllowed": true, // Allows changes of the value. similar to disable/enable
      "defaultValue": "standard" // If not configured, the first value of the enumeration is the default.
    },
    "mainPage": {
      "changeValueAllowed": true,
      "defaultValue": "non-standard-1"
    },
    "namespaces": {
      "12": {
        "changeValueAllowed": false,
        "defaultValue": "non-standard-2",
        "exclude": ["PageName"]
      },
      "14": {
        "changeValueAllowed": true,
        "defaultValue": "wide"
      }
    },
    "querystrings": {
      "action": {
        "changeValueAllowed": true,
        "values": {
          "view": "standard",
          "edit": "non-standard-2",
          "history": "non-standard-1"
        }
      },
      "diff": {
        "changeValueAllowed": true,
        "matchPattern": ".+",
        "defaultValue": "non-standard-3"
      }
    },
    "pagetitles": {
      "Special:Preferences": {
        "changeValueAllowed": true,
        "defaultValue": "non-standard-1"
      },
      "Special:Search": {
        "changeValueAllowed": false,
        "defaultValue": "standard"
      }
    },
    "description": "Configuration settings structured to define feature compatibility and default values for main pages, namespaces, query strings, and page titles. Includes a system fallback to handle unconfigured features, ensuring broad default customisation with the flexibility to adapt."
  }
}

This gives us a set of flags that can be driven by key value rather than processing, and it can utilise the key "DefaultUserOptions" in skin.json or replace it.

Feedback is appreciated until our next SHDT.

At a high level, can someone explain why the feature options and feature configuration key are separate? Personally I would consider these exclusion options as properties of a feature. Currently, we have the following two keys in skin.json:

"VectorNightMode":...
"VectorNightModeOptions":...

It's more intuative to me to think of these as a single object (since they are related), e.g:

"VectorNightMode": {
    "options": {...}
};

Is this because of technical limitations? The separation can also lead to confusion, where the options and feature key can be named differently, like in the case of the "VectorMaxWidthOptions" and the "vector-limited-width" user setting, e.g:

"DefaultUserOptions": {
	"vector-limited-width": 1,
},
"VectorMaxWidthOptions":

I think we should also remove the suggested

"changeValueAllowed": true

as we never set a different default value expecting it to be changeable, so an updated version should be like:

"DefaultUserOptions": {
  "FeatureName": {
    "defaultValue": "standard",
    "mainPage": {
      "defaultValue": "non-standard-1"
    },
    "namespaces": {
      "12": {
        "defaultValue": "non-standard-2",
        "exclude": [
          "PageName"
        ]
      },
      "14": {
        "defaultValue": "non-standard-1"
      }
    },
    "querystrings": {
      "action": {
        "defaultValue": {
          "view": "standard",
          "edit": "non-standard-2",
          "history": "non-standard-1"
        }
      },
      "diff": {
        "matchPattern": ".+",
        "defaultValue": "non-standard-3"
      }
    },
    "pagetitles": {
      "Special:Preferences": {
        "defaultValue": "non-standard-1"
      },
      "Special:Search": {
        "defaultValue": "standard"
      }
    },
    "description": "Configuration settings structured to define feature compatibility and default values for main pages, namespaces, query strings, and page titles. Ensuring broad default customisation with the flexibility to adapt."
  }
}

The core functionality should return the defaultValue and an overrideDefaultValue, and as long the override parameter is empty it means just normal behaviour, and otherwise we utilise the exception handling workflow

Lastly for this

Is this because of technical limitations? The separation can also lead to confusion, where the options and feature key can be named differently, like in the case of the "VectorMaxWidthOptions" and the "vector-limited-width" user setting, e.g:

"DefaultUserOptions": {
	"vector-limited-width": 1,
},
"VectorMaxWidthOptions":

I agree, that we should standardise using either but not both, let me know if the snake case is preferable to the camel case, but we can change from one to another easily on the code without confusing the configuration.

I prefer camel case myself if it is a vote here is mine.

@Jdrewniak when we started the feature management system it was only based on user - whether they were anon or logged in. Over time our feature management got conflated with the concept of per page features. As a result of this, we now have a lack of common understanding about what a feature (see ticket in backlog [jon to replace with link later when he's found it]) . I think it's important we establish one before making any decisions here around architecture - personally I would advocate for creating a specification describing a new system that meets our new needs, and migrating to it over time.

On the short term, I should note VectorNightMode is temporary - it will be removed when the feature is completed. VectorNightModeOptions isn't.

Note about preference keys: these are typically hyphenated in MediaWiki,whereas configuration flags are snake-cased. Changing preferences is super disruptive to existing users so make sure not to do that as part of this task.

Change #1027479 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/skins/Vector@master] feature(POC:vector): Refactor for DefaultPageOptions for enhanced flexibility

https://gerrit.wikimedia.org/r/1027479

Change #1027483 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/skins/MinervaNeue@master] feature(POC:minerva): Adapt Vector config method for night mode

https://gerrit.wikimedia.org/r/1027483

Change #1026891 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/core@master] feature(POC:core-skin): Preserve configured overrides in clientprefs.js

https://gerrit.wikimedia.org/r/1026891

Hey @Mabualruz I'd like for us to talk about this in SHDT on Wednesday. Given the proximity to the night mode deploy, and the fragility of our feature management system(s) these changes seem like they could be significant and risky.

I'd like for us to think about what's the absolute minimum work we need to do here to address the primary user story which I believe is "Use a small font size for certain pages. Use standard for other pages."
Right now it seems like we only need the standard font size on: 0,4,12,118

This task also does not seem like it was ready for development/estimation given there are still significant unanswered "Open questions" so I'm moving this to back to to "Backlog" until we've got some clarity on those points.

Change #1027479 abandoned by Jdrewniak:

[mediawiki/skins/Vector@master] feature(POC:vector): Refactor ConfigHelper for Vector skin

Reason:

Abandoning per https://phabricator.wikimedia.org/T363834 being declined.

https://gerrit.wikimedia.org/r/1027479

Change #1027483 abandoned by Jdlrobson:

[mediawiki/skins/MinervaNeue@master] feature(POC:minerva): Adapt Vector config methods for Minerva night mode

Reason:

Abandoning as https://phabricator.wikimedia.org/T363834 was resolved.

https://gerrit.wikimedia.org/r/1027483