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

FIX Collapse the preview panel by default if no preview is available. #1305

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented May 4, 2022

This makes the editor experience more streamlined for DataObjects that can have a preview (and therefore the preview panel is present) but do not have a preview in the current context (e.g. an elemental block owned by a non-previewable DataObject).

Dependencies (won't work without these PRs)

Parent issue

This makes the editor experience more streamlined for DataObjects that
_can_ have a preview (and therefore the preview panel is present) but do
not have a preview in the current context (e.g. an elemental block owned
by a non-previewable DataObject).
@GuySartorelli GuySartorelli force-pushed the pulls/4.11/collapse-empty-previews branch from 40ef7a1 to 340a24c Compare May 4, 2022 02:06
@michalkleiner
Copy link
Contributor

Should it be collapsed or disabled? Sorry been a while for me — what happens when you open the panel and there's no preview available? Is there a 'No preview available' state? All good if that's already the case.

@GuySartorelli
Copy link
Member Author

what happens when you open the panel and there's no preview available? Is there a 'No preview available' state?

Yup! Theoretically at least. There's a related PR to this in the description that makes sure null or empty preview links will display the "no preview available" message. Unfortunately the classes that handle that are in CMS... we should move them to admin at some point.

Should it be collapsed or disabled?

I think it should be collapsed - the "no preview" message will display if users choose to expand it. This reflects the fact that the data object is previewable, there's just no preview for it in the current context.
That said, I'm not a UX expert by any means... if anyone has reasoning why it should be disabled I'm happy to make that change.

@GuySartorelli GuySartorelli changed the base branch from 1 to 1.11 May 5, 2022 00:29
@GuySartorelli
Copy link
Member Author

Changed base from 1 to 1.11 as this is a fix that should be included in the 1.11.0 release.

@sabina-talipova
Copy link
Contributor

Nice work! Do you need test coverage for this?

@GuySartorelli
Copy link
Member Author

I've created an issue (#1306 ) for adding test coverage for previews in admin - there currently aren't any in admin itself, so it will be quite a big task that I won't be able to get done in the current sprint.

@sabina-talipova sabina-talipova merged commit 37af114 into silverstripe:1.11 May 5, 2022
@GuySartorelli GuySartorelli deleted the pulls/4.11/collapse-empty-previews branch May 5, 2022 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants