Skip to content

BUGFIX: Resolve issue with custom data source displaying additional l… #3701

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

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

markusguenther
Copy link
Member

Previously, there was an issue with the custom data source where the secondary label, utilized with label and preview, was not being displayed correctly. This commit addresses the problem, ensuring proper alignment and display of the secondary label in conjunction with the label and preview elements. The implementation includes the use of a CSS grid layout to achieve consistent and accurate rendering of the custom data source information.

Fixes: #3675

Before:
Screenshot 2024-01-26 at 13 28 55

After with preview:
Screenshot 2024-01-26 at 14 13 09

After without preview and 3rd label:
Screenshot 2024-01-26 at 14 17 11

@markusguenther markusguenther changed the base branch from 9.0 to 7.3 January 26, 2024 13:23
@github-actions github-actions bot added Bug Label to mark the change as bugfix 9.0 7.3 and removed 9.0 labels Jan 26, 2024
@markusguenther
Copy link
Member Author

For easier testing here my DataSource:

<?php

namespace Neos\Demo\Service\DataSource;


use Neos\Flow\Annotations as Flow;
use Neos\Flow\Persistence\PersistenceManagerInterface;
use Neos\Neos\Domain\Service\UserService;
use Neos\Neos\Service\DataSource\AbstractDataSource;
use Neos\ContentRepository\Domain\Model\NodeInterface;

class EditorsDataSource extends AbstractDataSource
{

    /**
     * @var string
     */
    static protected $identifier = 'neos-demo-editors';

    /**
     * @Flow\Inject
     * @var UserService
     */
    protected $userService;

    /**
     * @Flow\Inject
     * @var PersistenceManagerInterface
     */
    protected $persistenceManager;

    /**
     * @param NodeInterface $node The node that is currently edited (optional)
     * @param array $arguments Additional arguments (key / value)
     * @return array
     */
    public function getData(NodeInterface $node = null, array $arguments = [])
    {
        $options = [];
        foreach ($this->userService->getUsers() as $user) {
			$options[] = [
                'value' => $this->persistenceManager->getIdentifierByObject($user),
                'label' => $user->getLabel(),

                // additional optional parameters:
                'secondaryLabel' => "works here",
                'tertiaryLabel' => "is a nice person",
                'preview' => 'data:image/svg+xml;base64,PHN2ZyB4bWxucz0naHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmcnIHZpZXdCb3g9JzAgMCA0NDggNTEyJz48IS0tIUZvbnQgQXdlc29tZSBGcmVlIDYuNS4xIGJ5IEBmb250YXdlc29tZSAtIGh0dHBzOi8vZm9udGF3ZXNvbWUuY29tIExpY2Vuc2UgLSBodHRwczovL2ZvbnRhd2Vzb21lLmNvbS9saWNlbnNlL2ZyZWUgQ29weXJpZ2h0IDIwMjQgRm9udGljb25zLCBJbmMuLS0+PHBhdGggZD0nTTIyNCAxNmMtNi43IDAtMTAuOC0yLjgtMTUuNS02LjFDMjAxLjkgNS40IDE5NCAwIDE3NiAwYy0zMC41IDAtNTIgNDMuNy02NiA4OS40QzYyLjcgOTguMSAzMiAxMTIuMiAzMiAxMjhjMCAxNC4zIDI1IDI3LjEgNjQuNiAzNS45Yy0uNCA0LS42IDgtLjYgMTIuMWMwIDE3IDMuMyAzMy4yIDkuMyA0OEg0NS40QzM4IDIyNCAzMiAyMzAgMzIgMjM3LjRjMCAxLjcgLjMgMy40IDEgNWwzOC44IDk2LjlDMjguMiAzNzEuOCAwIDQyMy44IDAgNDgyLjNDMCA0OTguNyAxMy4zIDUxMiAyOS43IDUxMkg0MTguM2MxNi40IDAgMjkuNy0xMy4zIDI5LjctMjkuN2MwLTU4LjUtMjguMi0xMTAuNC03MS43LTE0M0w0MTUgMjQyLjRjLjYtMS42IDEtMy4zIDEtNWMwLTcuNC02LTEzLjQtMTMuNC0xMy40SDM0Mi43YzYtMTQuOCA5LjMtMzEgOS4zLTQ4YzAtNC4xLS4yLTguMS0uNi0xMi4xQzM5MSAxNTUuMSA0MTYgMTQyLjMgNDE2IDEyOGMwLTE1LjgtMzAuNy0yOS45LTc4LTM4LjZDMzI0IDQzLjcgMzAyLjUgMCAyNzIgMGMtMTggMC0yNS45IDUuNC0zMi41IDkuOWMtNC44IDMuMy04LjggNi4xLTE1LjUgNi4xem01NiAyMDhIMjY3LjZjLTE2LjUgMC0zMS4xLTEwLjYtMzYuMy0yNi4yYy0yLjMtNy0xMi4yLTctMTQuNSAwYy01LjIgMTUuNi0xOS45IDI2LjItMzYuMyAyNi4ySDE2OGMtMjIuMSAwLTQwLTE3LjktNDAtNDBWMTY5LjZjMjguMiA0LjEgNjEgNi40IDk2IDYuNHM2Ny44LTIuMyA5Ni02LjRWMTg0YzAgMjIuMS0xNy45IDQwLTQwIDQwem0tODggOTZsMTYgMzJMMTc2IDQ4MCAxMjggMjg4bDY0IDMyem0xMjgtMzJMMjcyIDQ4MCAyNDAgMzUybDE2LTMyIDY0LTMyeicvPjwvc3ZnPg=='
            ];
        }
        return $options;
    }
}

And the nodetype stuff:

properties:
    authors:
      type: array
      ui:
        label: 'Author(s)'
        reloadIfChanged: true
        inspector:
          group: 'document'
          position: '200'
          editor: Neos.Neos/Inspector/Editors/SelectBoxEditor
          editorOptions:
            placeholder: Choose
            dataSourceIdentifier: neos-demo-editors

Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

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

Thx, looks better (though I don't like the image hanging loose there, which is not a problem of this PR), but this change needs a cleaner CSS implementation.

@@ -1,8 +1,25 @@
.multiLineWithThumbnail__item {
display: grid;
grid-template-columns: auto 1fr;
gap: -var(--spacing-Half);
Copy link
Member

Choose a reason for hiding this comment

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

A negative gap is marked as invalid value and also doesn't seem to have any effect.

The styling would also be simpler when the gap is actually used and the padding of the element disabled and therefore the image correctly positioned (also the image margin needs to be disabled.

The result is looking the same but without all the CSS pushing around.

…abels incorrectly

Previously, there was an issue with the custom data source where the secondary label, utilized with label and preview, was not being displayed correctly. This commit addresses the problem, ensuring proper alignment and display of the secondary label in conjunction with the label and preview elements. The implementation includes the use of a CSS grid layout to achieve consistent and accurate rendering of the custom data source information.

Fixes: neos#3675
Long labels float out of the container and do not look nice. This adds an ellipsis and the tooltip is always available on hover to read the whole text. The Image is now also aligned in centered position.
@markusguenther
Copy link
Member Author

Thanks for the valuable feedback. With very long labels, it looked like that:
Screenshot 2024-02-02 at 13 35 23
Screenshot 2024-02-02 at 13 35 12

So the idea was to break the text, but it leads to huge heights.
Screenshot 2024-02-02 at 13 31 58

Now we add an ellipsis and, like in the NodeInfoView we have a tooltip by default.

Safari:
Screenshot 2024-02-02 at 14 18 28
Chrome:
Screenshot 2024-02-02 at 14 17 30
Firefox:
Screenshot 2024-02-02 at 14 17 18

@mhsdesign mhsdesign self-requested a review February 2, 2024 13:42
@neos-bot
Copy link

neos-bot commented Feb 2, 2024

🎥 End-to-End Test Recordings

These videos demonstrate the end-to-end tests for the changes in this pull request.

Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

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

Great much better!

margin-left: -var(--spacing-Full);
margin-top: -var(--spacing-Half);
margin-bottom: -var(--spacing-Half);
margin-left: -var(--spacing-Half);
Copy link
Member

Choose a reason for hiding this comment

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

Not so happy about still having a negative margin, but I think its fine for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but in the select boxes I am always afraid that I break something else 🙈

@markusguenther markusguenther merged commit 2d8485f into neos:7.3 Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.3 Bug Label to mark the change as bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Custom Data Source preview with label and secondaryLabel displays wrong
3 participants