backdrop-ops / docs.backdropcms.org

Website for displaying Backdrop CMS documentation and API source code.
https://docs.backdropcms.org/
6 stars 6 forks source link

Better Form API page #66

Closed ghost closed 4 years ago

ghost commented 5 years ago

A PR is available that improves the Form API page, and a Tugboat instance has been setup so you can see it in action.


The Form API page is currently a gigantic amount of code in a single Handbook Page node. This makes it really hard to update...

For example, I was just about to try adding the documentation for the URL type, but then I realised I would need to:

Ugh! Can't do it.

So that got me thinking, we need a better way of managing this page. I have two ideas:

  1. Setup content types (or vocabularies?) for the different pieces of information on this page (e.g. Elements, Properties) with reference fields to each other, then makes nodes for each bit of information and finally a view or custom code that pulls everything together to make this page.
  2. Put all the information from this page into a structured array in a config file, then write code to pull it all in and format the page.

I think 1. is more in line with how we do things in Backdrop, but it seems like overkill to me.

  1. would be essentially putting content into config, but then we want this page/content in the repo anyway, right?

I'm willing to give 2. a go if I can get confirmation that this is something worth trying and isn't totally the wrong approach.

ghost commented 5 years ago

This was discussed in one of the weekly meetings (https://youtu.be/dMDizzYs_R4?t=1076) and it was suggested that we use content types as that's the better way of doing it and because then the elements and properties from this page might be available to the search facility as well. I volunteered to play around with it and see what I could get working. @jenlampton provided me with a sanitised copy of the API site's DB, so I'll use that and report back when I've made some progress.

ghost commented 5 years ago

I've started work on this (finally) :tada: Will post feedback as I make some progress :wink:

jenlampton commented 5 years ago

@BWPanda I'm pretty sure that we won't be able to create a page that's as good as the one we have already using content and views. Your first attempt should be a proof-of-concept to see what's possible, don't spend too much time doing data entry until we're sure it will work...

That said, I just spent a few hours yesterday reworking the MASSIVE HTML FORMS and I would love if if this page were easier to maintain! :)

That said, it's more important to me that people be able to read/use this page easily than people be able to update this page easily.

Good luck! :) And thank you.

edit: oh also...

but then we want this page/content in the repo anyway, right?

I committed the HTML copy from the body of this page into the repo -- outside the docroot -- so that it will be easier to maintain. I was working with it as a .html file anyway, to make use of the syntax highlighting and auto-formatting from my editor.

jenlampton commented 5 years ago

@BWPanda pointed out that there was an older version of this page already in Git, so I've moved it (and the rest of the documentation directory) outside the docroot. I also replaced the old copy of the drupal form api page with my recent copy of the backdrop form api page. The backdrop verson contains only what's inside the Body field for easy copy/paste.

ghost commented 5 years ago

I've setup two content types with fields and a view, and as a result the main listing of elements and properties is working nicely so far. Currently overriding theme functions to make sure I can get things displaying as needed, then I'll look into the extra bits (matrix table, etc.).

jenlampton commented 5 years ago

The matrix table is the hard part, and is what will determine if this will work. Don't waste too much time on theme stuff until we get the whole proof of concept working.

ghost commented 5 years ago

Tada! Looks pretty good to me so far (less the styling obviously):

Screenshot - Form API Reference - Backdrop CMS API (api lndo site)

jenlampton commented 5 years ago

Wow, it looks great! I'm already impressed :) How are you planning on handling the Properties and Used by lines? Maybe views_field_view?

edit:

Properties on elements:

properties

Used by on properties:

used-by
ghost commented 5 years ago

The Properties line is already there for Elements, but for Properties then yes, views_field_view for Used by.

ghost commented 5 years ago

I'd like to know I'm doing things the 'right way' before going too much further, so what are your thoughts on Multifield vs. Paragraphs @jenlampton?

jenlampton commented 5 years ago

@BWPanda What architecture are you thinking about using? (what fields where, etc)

The main differences:

ghost commented 5 years ago
Element (content type):
  - Name (title)
  - Description (body)
  - Properties (multifield):
    - Property (node reference (Property))
    - Default value (textfield)
    - Recommended (checkbox)
  - Non-standard properties (textarea)
  - Usage examples (multifield):
    - Link (link)
    - Code (textarea)
    - Description (textarea)

Property (content type):
  - Name (title)
  - Description (body)
  - Values (textarea)
  - Usage examples (multifield (same as above))

One change I'm thinking of making is changing the Link field in the Usage Examples multifield to a textfield with formatting so the filter will automatically do the linking for us.

Other than that, mainly wanted to make sure people weren't going to say I should be using Paragraphs instead of multifield once I make my PR...

jenlampton commented 5 years ago

I don't think we need either paragraphs or multifields for either case.

The properties usage examples are too varied to split into individual fields, but the good news is that there's no reason to do so: they are never viewed independently. A longtext / body field should do the trick.

I'll look more at properties on elements, but why not a (node reference) checklist? I haven't yet seen an example with a default value, which element(s) is that coming from? (I couldn't find one)

Using a node reference checklist would enable the views-field-view functionality without needing to factor in a double relationship.

If that can be safely eliminated, a second identical (node reference) checklist could handle which ones are recommended, and some preprocessing could make those items in the first list bold.

On Wed, Nov 27, 2019, 6:07 PM Peter Anderson notifications@github.com wrote:

Element (content type):

  • Name (title)
  • Description (body)
  • Properties (multifield):
    • Property (node reference (Property))
    • Default value (textfield)
    • Recommended (checkbox)
  • Non-standard properties (textarea)
  • Usage examples (multifield):
    • Link (link)
    • Code (textarea)
    • Description (textarea)

Property (content type):

  • Name (title)
  • Description (body)
  • Values (textarea)
  • Usage examples (multifield (same as above))

One change I'm thinking of making is changing the Link field in the Usage Examples multifield to a textfield with formatting so the filter will automatically do the linking for us.

Other than that, mainly wanted to make sure people weren't going to say I should be using Paragraphs instead of multifield once I make my PR...

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop-ops/api.backdropcms.org/issues/66?email_source=notifications&email_token=AADBER7QJARJXTWH6RGBBIDQV4RV5A5CNFSM4IITMQU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFLGYYA#issuecomment-559311968, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER4F2YKI3CSPV5LK4DLQV4RV5ANCNFSM4IITMQUQ .

ghost commented 5 years ago

Good point about the usage examples; they could easily be a single textarea field. Not as strict as individual fields, but no worse than the current setup πŸ˜‰

As for Properties, I still think these need to be some sort of multifield thing. Default values can be seen here: https://api.backdropcms.org/form_api#element_default_values I haven't gotten to creating that part of the page yet, but shouldn't be hard. Unfortunately the default values are specific to elements, not properties (otherwise we could simply add that field to the Property content type).

I see your point about the checkboxes, but add in the default values and I think its easier to just have a multifield (in fact I can't think of any other way to do it...).

jenlampton commented 4 years ago

Default values can be seen here: api.backdropcms.org/form_api#element_default_values

Ah, thanks!

Unfortunately the default values are specific to elements, not properties

Hm... Why not a single longtext "Default values" field on Element? Again, these items are never viewed independently -- only ever as a complete UL. Why separate them into different places in the database only to combine them all back together again for the only place they are displayed?

ghost commented 4 years ago

Why not a single longtext "Default values" field

Because they are displayed separately as well:

Screenshot-20191130-055800-Brave.png

jenlampton commented 4 years ago

Ah! You know I saw the (default: X) in the lists of properties the other day, but couldn't figure out what that meant when it was in the middle of the list of properties. :)

I think it might make more sense if we displayed here in the same way we do when it's separate, like so:

Screen Shot 2019-11-29 at 8 28 49 PM
ghost commented 4 years ago

Ooh, interesting. I'll work on that then, with no need for multifield/paragraphs.

ghost commented 4 years ago

Actually, you'd still need multifield there to link the property name as a reference field and the default value separately...

jenlampton commented 4 years ago

Actually, you'd still need multifield there to link the property name as a reference field

If the whole ul is in the longtext rich-text field then the property name should be linked. It shouldn't link to the referenced node, it needs to link elsewhere on this same page: to the id property of the relevant #property section. I think we can do that using the Link button in the rich-text editor. If not, or if we can hand-write the HTML.

ghost commented 4 years ago

I don't know... If the default values section of an element is a textarea where you format your own list, links, etc. the the same could be true of the properties section just above it. And since the description and usage examples are already textareas, then the whole element could just become one big textarea. In which case how is that much better than what we have now?

jenlampton commented 4 years ago

I don't know... If the default values section of an element is a textarea where you format your own list, links, etc. the the same could be true of the properties section just above it.

True!

But adding new elements would be easier (editorially) if all available properties were a list of checkboxes you could choose to check or leave un-checked. It would prevent the problem of forgetting to add properties that should apply to to all elements. (I found some elements missing some properties when I made my last edits)

edit: This could be handled with a list-text field instead of a node reference, but we'd still need the properties sections to be populated somehow. If we have the nodes for those sections already, it would be nearly as easy (architecturally) to use a checkbox reference field.

And since the description and usage examples are already textareas, then the whole element could just become one big textarea.

Also true!

But it would be harder to format as one large text area than several smaller text areas.

In which case how is that much better than what we have now?

1) One element could be edited at a time, 2) HTML for one part of the page would be less likely to get broken from an error elsewhere 3) new items could be added easily 4) new items would automatically be added to one of tables at the top of the page

I'm pretty sure these are the only things we were trying to accomplish in the first place. And as an added bonus, using longtext fields would add one other benefit

5) it will be easier to copy what we have now into the new architecture

Can you point out any benefit of ~having property reference fields, or~ moving all these items into separate fields?

edit: Just answered my own reference question, above.

ghost commented 4 years ago

Can you point out any benefit of [...] moving all these items into separate fields?

The only reasons I see for not doing things the multifield/paragraphs way is 1. too hard and 2. takes longer. As for 1., if we can solve these problems now then it makes it easier for people using these modules in future (e.g. see https://github.com/backdrop-contrib/multifield/issues/34), and re. 2., the current system is still live and working and I see no reason to cut corners just to get something out the door quicker that may not be as effective, user-friendly and powerful as it could be otherwise.

klonos commented 4 years ago

@BWPanda where is the work for this happening? (I only have editor role on api.b.org, so limited access to certain content types).

jenlampton commented 4 years ago

@klonos I think there's a feature branch?

The only reasons I see for not doing things the multifield/paragraphs way is 1. too hard and 2. takes longer.

@BWPanda You're forgetting the most important reason: not being able to get as useful a page as we have now. It doesn't matter how easy it is for us to maintain a feature if nobody is using the feature in the first place.

The feature we have now works. Trying to rebuild the same thing "the drupal way" with nodes & views has not yet been proven possible -- though we are extremely close :)

ghost commented 4 years ago

Back to working on this, just FYI.

ghost commented 4 years ago

Finished a significant amount of work on this, enough to make a public PR: https://github.com/backdrop-ops/api.backdropcms.org/pull/87

Here's what it looks like: Screen Shot 2020-08-20 at 22 13 20-fullpage

Still some things to do, primarily get the table headers slanted. Also, there are hidden permalinks next to each heading - when you hover over it the permalink appears. Thought that looked better than headings that look like links but aren't...

docwilmot commented 4 years ago

Satisfy my curiousity please: In summary, two new node types, Paragraphs and References and templates and CSS, right? Why was Paragraphs needed? Did you already parse the file into individual nodes?

ghost commented 4 years ago

@docwilmot Good point, a summary is in order...

Here's what I did:

Here're the content types I setup:

And here's the paragraph I setup:

And here's what the forms look like: Element element_form Property property_form

ghost commented 4 years ago

Did you already parse the file into individual nodes?

No, this'll need to be done manually as far as I'm aware, but I'm happy to do that (I think there'll be some cleanup needed at the same time).

jenlampton commented 4 years ago

Love it!

On Thu, Aug 20, 2020, 4:21 PM Peter Anderson notifications@github.com wrote:

Did you already parse the file into individual nodes?

No, this'll need to be done manually as far as I'm aware, but I'm happy to do that (I think there'll be some cleanup needed at the same time).

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop-ops/api.backdropcms.org/issues/66#issuecomment-677953549, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER3M5VHA5LJNTHGTRQTSBWVWNANCNFSM4IITMQUQ .

ghost commented 4 years ago

Anyone know of an easy way we can setup a sandbox site for this so everyone can see/test without having to do it themselves?

jenlampton commented 4 years ago

It would be cool if we could use tugboat for our own websites.

On Thu, Aug 20, 2020 at 5:31 PM Peter Anderson notifications@github.com wrote:

Anyone know of an easy way we can setup a sandbox site for this so everyone can see/test without having to do it themselves?

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop-ops/api.backdropcms.org/issues/66#issuecomment-677972871, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER6HZGR74NMUGEQVALDSBW56VANCNFSM4IITMQUQ .

ghost commented 4 years ago

Ok, so I was able to setup a Tugboat instance just for this PR. You can see the new page in action here: https://master-uwyj4f73purhtdhvrvtxjijdaoees0qo.tugboat.qa/form-api

If you'd like to login to have a play (e.g. add new Properties or Elements, edit existing ones, etc.), you can do so with: Username: drop Password: BackdropCMS

As the PR gets updated with further improvements, I'll update that Tugboat site so people can see the progress and test changes, etc.

ghost commented 4 years ago

I've tried looking at getting the table headers rotating properly again, but it's beyond me. @quicksketch did this originally with what appears to be custom JS code (I can't find any references to where he may have gotten this from), so maybe he needs to take a look and see what needs changing to get it working on the new table...?

Other than that, I think we just need people to check out the new look and provide feedback so we know what else needs doing before merging this and setting up the new page on the live site.

ghost commented 4 years ago

FTR, I tried adding this line to the JS file:

// Add the data-rotate attribute.
$(context).find('table.form-api thead tr').attr('data-rotate', '-45');

directly above this line:

// Rotate table headers 45 degrees. CSS added in style.css.
$(context).find('tr[data-rotate]').once('rotate').each(function() {

And that does rotate the headers, but there's a positioning issue I can't figure out...

image

jenlampton commented 4 years ago

I think maybe we can do this with CSS now. Did you try transform: rotate()? I have this on one of my other sites and it works okay.

    -webkit-transform: rotate(300deg);
    -moz-transform: rotate(300deg);
    -o-transform: rotate(300deg);
    -ms-transform: rotate(300deg);
    transform: rotate(300deg);
    vertical-align: middle;
ghost commented 4 years ago

Thanks @jenlampton. That code didn't work verbatim, but I played around with it and, with the help of a codepen I found, I was able to get it working like this: image

There're some CSS values that'll need tweaking once all the data's been entered in to make sure everything fits nicely, but I think that's easy enough to do.

Because I had to rebuild the Tugboat instance, the URL changed. You can access it now at: https://master-ajpft2btfrhx5lb61aafpewfwzvsnvsl.tugboat.qa/form-api

jenlampton commented 4 years ago

Thank you for working on this, I am impressed by your persistence :)

ghost commented 4 years ago

What are the next steps? Do we need more people to test this, or, knowing that this can replace all the functionality of the current page, can we just implement it on the live site, create the content, and then iteratively fix bugs as we notice them?

jenlampton commented 4 years ago

We'll need to set up the content types and create all the content first, then later swap out the old HTML page with the new view page.

ghost commented 4 years ago

@jenlampton I've made two final changes: one to makes sure the new CSS only applies to the new page (so the existing page isn't messed up and both can work at the same time), and two to make the new view only visible to Developers (I'll make this public once the content's created and it's ready to go). The content types are in config (part of the PR), so can I push this live and start adding content now?

jenlampton commented 4 years ago

Sounds great!

On Wed, Aug 26, 2020, 3:00 PM Peter Anderson notifications@github.com wrote:

@jenlampton https://github.com/jenlampton I've made two final changes: one to makes sure the new CSS only applies to the new page (so the existing page isn't messed up and both can work at the same time), and two to make the new view only visible to Developers (I'll make this public once the content's created and it's ready to go). The content types are in config (part of the PR), so can I push this live and start adding content now?

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop-ops/api.backdropcms.org/issues/66#issuecomment-681146127, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER2WXBIQF42FMCLZWHDSCWAZXANCNFSM4IITMQUQ .

jenlampton commented 4 years ago

πŸŽ‰

On Wed, Aug 26, 2020, 5:42 PM Peter Anderson notifications@github.com wrote:

Closed #66 https://github.com/backdrop-ops/api.backdropcms.org/issues/66 via #87 https://github.com/backdrop-ops/api.backdropcms.org/pull/87.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop-ops/api.backdropcms.org/issues/66#event-3697099800, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER4SVETXSQP7OOZHCCDSCWTY3ANCNFSM4IITMQUQ .

ghost commented 4 years ago

I've merged my work into the main branch (forgot to 'squash' first, sorry) so the live site has this feature now. I'll be adding content over the next day or so and, when ready, will take the new page live. Any further requests/bugs with this can be opened as new issues.