Skip to content

FEATURE: Add inspector default group #3426

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

Open
wants to merge 1 commit into
base: 8.4
Choose a base branch
from

Conversation

mhsdesign
Copy link
Member

Bildschirm­foto 2023-03-10 um 22 30 15

Neos part: neos/neos-development-collection#4068

Resolves: neos/neos-development-collection#4060

What I did
Use the same code like for the default tabs, but also check if an editor is set as always properties will be shown which dont have an editor configured

How I did it

How to verify it

@mhsdesign mhsdesign requested a review from mficzel March 10, 2023 21:34
@github-actions github-actions bot added Feature Label to mark the change as feature 8.3 labels Mar 10, 2023
@mficzel
Copy link
Member

mficzel commented Mar 13, 2023

Could confirm that this works with properties that have an explicit editor configuration.
However properties that get the inspector configuration from the DefaultPropertyEditorPostprocessor are not affected.

Not yet sure what would be right but somehow think this should show up in the inspector:

  properties:
    text:
      type: string
      ui:
        label: A text field

This may be a naive assumption and could lead to all inline fields showing up in the inspector aswell which would be clearly wrong.

@mhsdesign
Copy link
Member Author

mhsdesign commented Mar 13, 2023

Huh that’s interesting (thx for testing this so well :D)

I indeed implemented the check for the editor field, as otherwise all fields are shown in the inspector.

i don’t know yet how this correlates to the php nodeType processor (as it is in php and should have been already applied so the editor key should be set nevertheless but I might be wrong or haven’t looked in the right place )

@crydotsnake crydotsnake changed the title Feature: Add inspector default group FEATURE: Add inspector default group Mar 14, 2023
@Sebobo
Copy link
Member

Sebobo commented Mar 15, 2023

I don't think fields without any kind of editor or inspector properties should show up. At least not with this initial change.
I know quite a bunch of nodetypes which have some properties from imported data and with labels to explain a bit better what they are for. But they should not be visible to editors.

@mhsdesign
Copy link
Member Author

Yes youre right. @mficzel and me just discussed this in the weekly and we agreed to close this one as there is no good way in knowing whether the property is intendet to be shown or not.

So until we found better criteria ill close this okay? (The neos part, that we even have a default group, which can be used out of the box will stay - so its at least half of a win ^^)

@mhsdesign mhsdesign closed this Mar 15, 2023
@mhsdesign mhsdesign reopened this May 15, 2023
@mhsdesign
Copy link
Member Author

I just talked to @Sebobo about this and he is still a fan of this idea and mentioned: one can still use the option hidden: true instead of assigning no group to the property.

What do you think @mficzel ?

so we would need to write instead of:

_path:
    type: string
_name:
    type: string

this to hide the properties explicitly:

_path:
    type: string
    ui:
      inspector:
        hidden: true
_name:
    type: string
    ui:
      inspector:
        hidden: true

we could even try to provide a migration for this, or projects would see popping up previously hidden properties ...

@markusguenther markusguenther force-pushed the feature/inspectorDefaultGroup branch from 876185a to 4a2b8fe Compare December 28, 2023 13:00
@markusguenther markusguenther changed the base branch from 8.3 to 8.4 December 28, 2023 13:00
@github-actions github-actions bot added 8.4 and removed 8.3 labels Dec 28, 2023
@markusguenther
Copy link
Member

Rebased the PR to 8.4 :)

@mficzel
Copy link
Member

mficzel commented Jan 13, 2025

Using explicit hidden:true instead of non existing groups sounds like the right way to me.

Will need some documentation an I would consider this breaky and prefer a major release for that

Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by testing and discussing. Cannot say much about ts

@Sebobo
Copy link
Member

Sebobo commented Jan 13, 2025

There are integrators who use a non-existent group to hide stuff, I think even with a condition.

So the question would be: Should we have a third state: Match, Null, Non-existent group and hide that one still?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.4 Feature Label to mark the change as feature
Projects
Status: In Progress 🚧
Development

Successfully merging this pull request may close these issues.

FEATURE: Define default group for inspector properties
4 participants