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

Add FormBuilderLoader in-line on blocks #327

Closed
10 tasks done
ScopeyNZ opened this issue Aug 15, 2018 · 12 comments
Closed
10 tasks done

Add FormBuilderLoader in-line on blocks #327

ScopeyNZ opened this issue Aug 15, 2018 · 12 comments

Comments

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Aug 15, 2018

The designs show a collapsible section where a form will appear when expanded:

https://projects.invisionapp.com/share/ZRL4T2HQAGB#/screens/301923214

image

Additionally a custom React form component will have to be used as we can't use form tags - we're rendering within the top-level page form

A/Cs:

  • Implement a collapsible section per the designs
  • On expand - load a form with FormBuilderLoader using the schema endpoint (implemented by Add a form schema endpoint for elemental to return blocks #326)
  • Expanding and collapsing the block is keyboard accessible.
  • The form is keyboard accessible
  • Form is not removed when collapsed
  • Form is not reloaded on subsequent expand
  • A custom form react component is used (by setting the SchemaComponent on a form) to remove the use of a <form> tag

Split from #295
Expanding functionality has been tasked out silverstripe/silverstripe-elemental-blocks#43

Pull requests

@brynwhyman
Copy link

brynwhyman commented Aug 20, 2018

This looks like it could replace the placeholder issue we created for adding expand functionality: silverstripe/silverstripe-elemental-blocks#43

If so, I'll update the ACs here

Keeping expand functionality separate in: silverstripe/silverstripe-elemental-blocks#43

@brynwhyman brynwhyman added Epic and removed Epic labels Aug 20, 2018
@ScopeyNZ
Copy link
Contributor Author

Ah yes. I was looking for that. It's on a different repo which could explain why I wasn't able to find it.

@newleeland
Copy link

newleeland commented Aug 20, 2018

Additional design reference (Style guide)
https://invis.io/7ZNLP773RSY (design in first post is up-to-date)

Complete blocks style guide
Blocks: https://invis.io/T7NLP7WP3K4
Layout blocks: https://invis.io/BNNLP87VQJU

@raissanorth
Copy link
Contributor

The Behat tests will need to have a scenario added that checks if the element is editable. This has been removed in #353, as at that point in time only a placeholder was rendered.

@robbieaverill
Copy link
Contributor

I’m adding a class when the element is not editable at the moment as part of another ticket. You think that’ll be ok? I’ll update the behat test to cover the dropdown menu being there or not. Maybe will add userforms to the tests as a concrete element which doesn’t allow it

@NightJar
Copy link
Contributor

NightJar commented Sep 4, 2018

Required to resolve this issue:

@rupachup rupachup modified the milestones: Sprint 20, Sprint 21 Sep 4, 2018
@robbieaverill
Copy link
Contributor

robbieaverill commented Sep 5, 2018

I've just given it a test run - some comments:

Otherwise it's looking great and behaving exactly as I'd expected it should, e.g. the focus on each element is affecting the drop shadow, the form is able to be expanded and collapsed without reloading the form, tabs work individually and the TinyMCE implementation seems to be working without bleeding into separate instances on the same page.

@NightJar
Copy link
Contributor

NightJar commented Sep 5, 2018

Yes there is some minor issue with TinyMCE. Simply clicking the area under the label will trigger the render as well (weirdly, the same is not true if one clicks where TinyMCE should be). It's a rather bizarre.

I really don't know how to get around this without spending a whole lot of time porting the entire thing (i.e. our plugins) to react - as per silverstripe/silverstripe-admin#623

To be clear: this issue does not happen when using the TinyMCE thin wrapper react component.
... but then the insert link/media dialog doesn't load (but the buttons on the toolbar do).

@robbieaverill
Copy link
Contributor

To be fair, implementing TinyMCE in React wasn't part of the scope of this ticket. I think what you've done is more than enough for now

@NightJar
Copy link
Contributor

OK, I figured out what triggered the initial render - it must be an Entwine event.
E.g. clicking where the editor should have been is a native event, hovering the site tree activates an entwine hover event & causes the render.

https://github.com/hafriedlander/jquery.entwine/blob/master/src/domevents/jquery.entwine.domevents.addrem.js#L132-L134

Luckily I was able to track down a way to manually trigger entwine into updating, and have updated the admin PR with such 💥

@NightJar
Copy link
Contributor

Thanks @ScopeyNZ @robbieaverill and @raissanorth for helping me get this through!

@robbieaverill
Copy link
Contributor

Nice job!

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

7 participants