Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS items should only belong to a single group #238

Open
wbamberg opened this issue May 29, 2018 · 8 comments
Open

CSS items should only belong to a single group #238

wbamberg opened this issue May 29, 2018 · 8 comments
Labels
idle Issues and pull requests with no activity for three months. question

Comments

@wbamberg
Copy link
Contributor

wbamberg commented May 29, 2018

@rachelandrew is working on sorting out the MDN docs on grid/flexbox/box alignment, and pointed out that the sidebars for these items don't make sense. It seems that part of the reason for this is that some of these items are marked as belonging to multiple "groups" in the metadata. I'd like to propose that we change the data so as to only allow an item to belong to a single group.


CSS data contains a property groups. The docs say:

groups (array of unique strings with at least 1 entry): CSS is organized in modules like "CSS Fonts" or "CSS Animations". MDN organizes features in these groups as well — groups should contain the name of the module(s) the property is defined in.

In most cases, groups only contains a single item, but in a few cases it contains more than one. For example, break-after:

"groups": [
    "CSS Columns",
    "CSS Fragmentation",
    "CSS Regions"
]

Unfortunately the macro used to build CSS sidebars on MDN, CSSRef.efs, isn't very good at dealing with this situation, and the sidebars tend to come out incoherent. Here's the structure of the sidebar for column-gap, with annotations to show its problems:

    CSS
    CSS Reference
    CSS Columns                                  <- this menu item has no children
    CSS Box Alignment
    -> Guides
        -> Using multi-column layouts            <- this should not be under box alignment
        -> Box Alignment In Grid Layout
        -> Box Alignment in Flexbox
        -> Box Alignment in Multi-column Layout
    -> Properties
        -> break-after
        -> break-before                          <- this should not be under box alignment, but it's here because of [this](https://github.com/mdn/data/blob/master/css/properties.json#L3315)
    ...
        -> column-count                          <- neither should this
    ...
        -> column-gap (grid-column-gap)
    ...

We could fix the macro here to handle multiple group better, but I'm starting to think that we would be better to say an item can only belong to a single group. If an item can only belong to one group, then the semantics are clear: it belongs to the group (aka module) whose spec currently defines it. If its definition moves from one spec to another (as, for example, align-items moving from flexbox to box alignment) then the group changes.

We could do this in the following stages:

  1. clean the data so the groups array contains only one group. This would fix the sidebars, while not breaking any client which expects groups to be an array.
  2. add a new property module which is a single string, and deprecate groups.
  3. update any code that uses groups, to use module instead
  4. remove groups

What do you think?

  1. do you think only having one "group" is a good change?
  2. do you currently use groups, and if so, are you able to update?
@wbamberg wbamberg changed the title Should CSS items only belong to a single group? May 29, 2018
@teoli2003
Copy link
Member

The underlying question is: is there a 1-n relationship between a group and properties. In other cases, does a property always belong to one single group?

To answer this, we first need a definition of what is a group. Is it a spec (with a CSS2 "supergroup") or is it something else? Like a group of properties that work well together and have a logical connection (with the hypothesis that a reader will often want to navigate from one to the other).

On the dual side here, the question is also what do we want from the sidebar? What are we trying to achieve there? I got anecdotical feedback (in opposition to my gut feeling) that we should just list all properties as we never get them right. Is this correct? Have we conducted any user testing here?

I'm ok to change the schema here, but I think we should take the time of a thorough analysis + user-testing to get the sidebar right. Once we know what we want, the "how" should be pretty straightforward.

@rachelandrew
Copy link
Collaborator

rachelandrew commented May 30, 2018

In CSS things belong to one module, Box Alignment - which is where all this comes up - defines things like the alignment properties which were originally part of flexbox, but the flexbox spec tells people to refer to box alignment in future.

The gap properties - were in grid and multicol, are now specified in Box Alignment. I think it makes sense that we have these things in a Box Alignment group, and then use other methods to explain the relationship between specs. Otherwise we get the confusing state that column-gap is in right now https://developer.mozilla.org/en-US/docs/Web/CSS/column-gap with a collection of things from multicol, box alignment and fragmentation. If the user has come from looking at CSS Grid, what relevance do fragmentation properties have?

@wbamberg
Copy link
Contributor Author

I think we should take the time of a thorough analysis + user-testing to get the sidebar right

I agree with this, but doing "a thorough analysis + user-testing" should not block the proposal made above. We don't have any current plans or resources to rework sidebars from the ground up, so there's no date we can expect to have it done by. Meanwhile the CSS sidebars on MDN are rather broken in a lot of instances.

So I think we should make a fix to the sidebars that keeps the same basic conception and infrastructure, and decouple that from a ground-up redesign.

I can think of one alternative to the proposal here: fix the csssyntax macro so it does a better job of representing multiple groups. I think this would be OK, but it's not my recommendation for a couple of reasons:

  1. it's more work (although still not a great deal of work)
  2. more importantly, it's conceptually unclear what the "multiple groups" thing means at the moment, and this I think leads to sidebars that present what seems to be a strange collection of items (as column-gap does now, for instance). Saying that the "group" just is the spec that defines the item means it's very clear what we should put in there, so we're more likely to get consistent and correct data.
@chrisdavidmills
Copy link
Contributor

The proposal specified here makes sense to me, at least as a short term solution to the sidebars being somewhat broken. In the future, we can then also go forward and work out a plan for updating those sidebars further, with user testing, etc.

My only comment is - are there any instances left in CSS where things are not divided up cleanly into modules, and we'd want to have the things in a certain sidebar to not just relate cleanly to the whole of a module? Anything older, from CSS2 or whatever? If this was the case, then perhaps we'd need to rename "groups" to "group" instead of "module", to allow for such an eventuality? The specified group would usually be just a module, but sometimes wouldn't be? I think this is basically @teoli2003 's concern too, but I agree we should think about it a little before proceeding.

@rachelandrew
Copy link
Collaborator

I think pretty much things that are defined in CSS2 are also mentioned in another module (even if the definition needs referring back to the CSS2 spec). I might be wrong but I can't think of anything obvious.

I think if we can make them a bit better in the short term, then we take a look at them as a whole and see what an ideal solution would look like and test that.

@chrisdavidmills
Copy link
Contributor

OK, let's go for it then. Even if we did suddenly come up against something very specific that doesn't fit neatly in a module, we can always make up a name for it like "CSS2-featureblah" or something.

@wbamberg
Copy link
Contributor Author

wbamberg commented Jun 1, 2018

Part 1: #239

@a2sheppy
Copy link
Contributor

a2sheppy commented Jun 6, 2018

I'm unclear reading this what the final proposal was. My thought is that we should do this:

Change "groups" to accept either a single string identifying the one group an item belongs to, or an object like this:

  "groups": {
    "primary": "<primary-group-name>",
    "other": <one-or-more-additional-groups>
  }

We can then use the primary group name when building the sidebar, while still offering the other group or groups (defined as a string or array of strings) for additional information. For instance, the sidebar could be revised to list "Related technologies:" followed by links to the secondary groups, if any.

That would very much simplify cleanup of the sidebar while allowing the addition of extra value at the same time. Not to mention establishing an explicit connection to the specification.

Then add a "specification" field, which takes a single string identifying the CSS specification that defines the thing. This string should match what we use in the rest of our databases.

@github-actions github-actions bot added the idle Issues and pull requests with no activity for three months. label Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idle Issues and pull requests with no activity for three months. question
5 participants