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

More options for content block preview (redirect) #269

Closed
12 tasks
brynwhyman opened this issue Jul 19, 2018 · 9 comments
Closed
12 tasks

More options for content block preview (redirect) #269

brynwhyman opened this issue Jul 19, 2018 · 9 comments

Comments

@brynwhyman
Copy link

brynwhyman commented Jul 19, 2018

User Story

As a CMS user, I want to be able to access content block options from a block, so that I can do this while reviewing the block at a page level.

Subtasks

Acceptance Criteria

  • Content blocks have a clickable ‘more options’ icon, as per the wireframe: https://projects.invisionapp.com/share/ZRL4T2HQAGB#/screens/301923101
  • Clicking the icon presents a popup with the following (clickable) options:
  • Clicking 'Content' takes user to existing content page tab
  • Clicking ‘History’ takes user to existing History page tab
  • Clicking ‘Settings’ takes user to existing Settings page tab
  • Any other tabs that are top level are also included and link to existing functionality, e.g link tracking
  • Mouse hover styling is present for options in popup
  • More options icon is still available when the block is expanded (as per Expand block previews silverstripe-elemental-blocks#43)

Notes

@robbieaverill
Copy link
Contributor

We are going to take a little bit of a performance hit by having to load all the CMS form fields for every block when you load a page in the CMS.

Relevant comment from @chillu on recent work for a similar feature: symbiote/silverstripe-gridfieldextensions#225 (comment)

@chillu
Copy link
Member

chillu commented Aug 6, 2018

Thanks for flagging this Robbie! Can we please measure before-after state on a non-trivial page (e.g. using ten instances of five different blocks)? In general, we should move this to a client-side cache which gets refreshed in the background, for the (usually small) chance that the form fields on this particular record are different from the ones we've cached. In which case React can re-render the fields. We've split out structure from data in the form schema to allow for this, right? There's potential for edge cases, where you started editing a form field which is then removed in the re-render (because of conditional logic in getCMSFields().

Ideally this should happen via GraphQL, which comes built-in with powerful client caching (and invalidation). Which means getting form schemas through GraphQL, and the "form registry" we're using for AssetAdmin.

Again, outlining the long term vision here, let's try to get as close as possible to this. OSSers team is happy to help with any GraphQL form schema work if that's required as a baseline.

@ScopeyNZ ScopeyNZ removed their assignment Aug 9, 2018
@ScopeyNZ
Copy link
Contributor

So I've been thinking a little about this. The performance hit for only 3 blocks is pretty severe - about 20% of the request is spent on getCMSFields. This would be way higher for a full page of blocks - and that didn't include more complex blocks like elemental userforms (although this isn't intended to be edited in-line). I don't think we can realistically load tab names for each block on the page without a significant hit to performance.

There could be a compromise though - we can load and cache tab names per block type. This would give a list of options that's accurate most of the time. In the rare case that the state of an element affects the tabs that it shows, there is a hook that we can use in FormBuilderLoader to update the options - the downside being that the user would have to trigger in-line editing by either expanding the block or clicking one of the existing options.

End result:

  • Performance not affected when loading tab names from a cache
  • List of tab options may not be complete until the form is loaded for a block
  • Once the form is loaded the block will maintain an updated list of tab options
  • If the user manages to remove tab options from modifying block data, the options should automatically update.
  • Eliminates the need for an additional request when a block is added

@clarkepaul wasn't keen on just having some of the tabs as options, so hopefully this is an acceptable compromise.

There is one gotcha here. The way that "many" relations add tabs (eg. Link Tracking) means that we need an existing object to fetch form fields for, or we can do this hacky thing:

$object = self::singleton();
$object->ID = 1;
$fields = $object->getCMSFields();

This works and gives the right list of tabs though!

@robbieaverill
Copy link
Contributor

there is a hook that we can use in FormBuilderLoader to update the options - the downside being that the user would have to trigger in-line editing by either expanding the block or clicking one of the existing options

AFAIK you will need to expand the block in order to edit it, it'll show a preview by default.

The way that "many" relations add tabs (eg. Link Tracking) means that we need an existing object to fetch form fields for, or we can do this hacky thing:

Note that the presence of these two tabs has been raised as a core bug: silverstripe/silverstripe-cms#2227

@ScopeyNZ
Copy link
Contributor

AFAIK you will need to expand the block in order to edit it, it'll show a preview by default.

This is true, but it appears the edit options still appear in the list even when you don't have the form loaded. If the A/C means the form has to be loaded before we get those options in the dropdown then that's ideal.

@robbieaverill
Copy link
Contributor

I don't think that sounds like it makes sense =(

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Aug 13, 2018

The tabs don't show as tabs with this, the tab content is shown when the section is chosen from the dropdown. The dropdown is rendered regardless of whether the form is rendered. I assume when you click the tab option (eg. Content) the form will be loaded and that tab content will be shown. Maybe @clarkepaul can clarify.

@ScopeyNZ
Copy link
Contributor

The designs answered the question:
image

We should have the tab options before we load the form.

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Aug 15, 2018

I have split this into a few issues: #323, #324 and #325 (noted above). Closing this as it's split

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants