Preview Conditions define what filters can be used to set the preview mode. Preview Conditions will expose the input form for authors to select the criteria (e.g. a date widget for selecting preview dates), and interact with the Site State Object to store and retrieve the conditions for the preview mode. The Preview Condition will not store the data themselves, only interface with the data.

PreviewCondition
Stores the values for the condition. It's composed of a PreviewConditionController.
PreviewConditionCollection
Provides functionality on a group of Preview Conditions

PreviewCondition (Interface)

  • __construct(PreviewConditionController)
  • getPreviewForm($form, $form_state) - Get the FormAPI preview form for the condition. Return an array
  • extractValues($values) - Extracts the values for the conditions from the FormAPI array.
  • validatePreviewForm($form, $form_state) - Validates the preview form.

PreviewConditionCollection

  • getPreviewForm($form, $form_state) - Gets the FormAPI for the preview form. Delegates to each PreviewCondition in the collection
  • validatePreviewForm($form, $form_state) - Validates the preview form. Delegates to each PreviewCondition
  • extractValues($values) - Extracts the values for the conditions from the FormAPI array. Delegates to each PreviewCondition
CommentFileSizeAuthor
#11 lsd-csi-1623104-conditions-interface.patch47.13 KBjec006
#10 csi-1623104-override-and-widget-2.patch9.62 KBjec006
#8 csi-1623104-override-and-widget-interfaces.patch2.47 KBjec006
#6 classes2.pdf54.08 KBjec006
#4 classes.pdf44.14 KBindytechcook

Comments

indytechcook’s picture

indytechcook commented
Issue tags: +lsd-csi

Add lsd-csi tag

indytechcook’s picture

indytechcook commented
Issue tags: +lsd-csi sprint 1

Add tags and assigned

indytechcook’s picture

indytechcook commented
Assigned: Unassigned » jec006

Assigning to josh.

This must utilize the plugin system also.

indytechcook’s picture

indytechcook commented
StatusFileSize
new classes.pdf44.14 KB

Here is a email conversation that had occurred between Josh, Eric and myself.

Josh put up the classes.pdf.

Response from Erik

So, I think maybe we can replace the whole form part with Field widgets. I feel like the fit the bill very well and they already exist in drupal.

I imagine That there would a be a set of preview config, that would say which condition should be avaiable, and with that which widget to use for the form. I think this would simply the class sturcture alot and would take the part that does not need to get pass around out of the class.

whatch think?

Response from Neil

That's really interesting. I think in theory it sounds good. We would probably just need to write an adapter around using the FieldAPI.

But it's very interesting indeed.

Response from Josh

So I've thought about this a fair amount and I have a few concerns:

  1. Field widget api is extremely closely coupled with field_attach_api. Specifically,
    • almost all the functions take a field as one of the parameters,
    • it doesn't have a form_submit action as a part of it (as it normally calls field_save hooks)
    • Field widgets are defined as being attachable to certain types of fields instead of by the data structure
  2. Field widget api doesn't provide any significant functionality over forms API ... which is to say, we don't necessarily gain much from using it other than that we don't have to define the forms themselves.
    • This saves us very little work as it seems most if not all of these forms would be 1-3 elements.

    3. So, I guess I would like to see significant benefit from field widget api if we're going to use it as in my view, we're mis-using it. I think we'll be better off allowing people to just use FAPI, widgets seems like they will lead to extra complexity, annoyance and weird bugs (as its extendable and we can't really know what people will do with it ... and their goals in adding field widgets are different than ours.)

    I thought about us maybe making these entities and then fielding them, but I think we gain nothing and lose alot of niceness if we do that.

indytechcook’s picture

indytechcook commented

Summary of IRC and Google Hangout conversations

  • FieldAPI/Widget API are to coupled to each other to really make any good use.
  • The Override and form will be associated to each other via a PreviewConfig. This needs to be documented
  • The Overrides doe not need to know anything about the Form. The Form will send the values to the overrides.
  • PDF will be updates
jec006’s picture

jec006 commented
StatusFileSize
new classes2.pdf54.08 KB

An updated classes diagram with some of the new ideas.

indytechcook’s picture

indytechcook commented

So this replaces Preview Conditions and Setter concepts correct? An others?

jec006’s picture

jec006 commented
StatusFileSize
new csi-1623104-override-and-widget-interfaces.patch2.47 KB

Here is an initial patch with the proposed interfaces for Widget and Override.

The classes diagrams are still undergoing work. I think the PreviewForm should really be called the condition, the Overrides replace the conditions. We're still working on figuring out how to handle new data types in the Override.

e2thex’s picture

e2thex commented

Here is the current working copy of the Class Diagam

            A-----------------.               I-----------------.
            |     Widget      |-------------->| WidgetInterface |
            '-----------------'               '-----------------'
                     |
             .-------'---------------.
             v                       v
.------------------------.  .-----------------.
| SelectWidgetCollection |  |  GenericWidget  |
'------------------------'  '-----------------'

           A-----------------.               I-------------------.
           |    Override     |-------------->| OverrideInterface |
           '-----------------'               '-------------------'
                    |
          .---------'--------------.
          v                        v
.------------------. .--------------------------.
| GenericOverride  | | EntityCollectionOverride |
'------------------' '--------------------------'

          A-----------------.               I--------------------.
          |    Condition    |-------------->| ConditionInterface |
          '-----------------'               '--------------------'
                   |
           .-------'-----------------.
           v                         v
 .------------------.  A--------------------------.
 | GenericCondition |  |     WrapperCondition     |
 '------------------'  '--------------------------'
                                     |
                                     v
                       .--------------------------.
                       |  SingleWrapperCondition  |
                       '--------------------------'

           A-----------------.               I-------------------.
           |    Reaction     |-------------->| ReactionInterface |
           '-----------------'               '-------------------'
                    |
              .-----'-------------------------.
              v                               v
 .------------------------.       .----------------------.
 |  EntityQueryReaction   |       | PageLoadNoadReaction |
 '------------------------'       '----------------------'
jec006’s picture

jec006 commented
StatusFileSize
new csi-1623104-override-and-widget-2.patch9.62 KB

Here is a patch with updated implementations of the interfaces, along with the beginnings of some code.

The plan is to try to implement a bit of the code to see what parts don't work - which is to say, work backwards a bit.

Still working on getting enough to have tests.

jec006’s picture

jec006 commented
Status: Active » Needs review
StatusFileSize
new lsd-csi-1623104-conditions-interface.patch47.13 KB

The work for this issue has been going on in the conditions branch. The interface work is complete for now (I'm sure there will be changes as we build out the basic pieces) so this issue is ready for review.

I've attached a patch which is the difference between the condition and 7.x-1.x branches.

indytechcook’s picture

indytechcook commented
Status: Needs review » Needs work

There is alot in this patch that is also in http://drupal.org/node/1623128#comment-6190426. I'm just commenting on the Condition part of this patch. I don't feel this will be an issue when committing since I will just merge the branches in git.

+++ b/sps/lib/Drupal/sps/Plugins/Condition/PreviewConditionIterator.phpundefined
@@ -0,0 +1,9 @@
+namespace Drupal\sps\PreviewCondition;
+
+use IteratorAggregate;
+
+class PreviewConditionIterator {//@TODO: implements  IteratorAggregate {
+

Do you like the name "PreviewConditionIterator" or "PreviewConditionCollection"? I've been going back and forth.

indytechcook’s picture

indytechcook commented
Status: Needs work » Fixed

Merged this code and create #1672622: Review name of the PreviewCondition Class to follow up on the naming convention.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Update Summary