Skip to content

Conversation

@mkoistinen
Copy link
Contributor

This templatetag acts like the templatetag 'render_model_block' but with a
plugin instead of a model as its target. This is used to link from a block of
markup to a plugin's changeform in edit/preview mode.

This is useful for user interfaces that have some plugins hidden from display
in edit/preview mode, but the CMS author needs to expose a way to edit them.
It is also useful for just making duplicate or alternate means of triggering
the change form for a plugin.

This would typically be used inside a parent-plugin’s render template. In this
example code below, there is a parent container plugin which renders a list of
child plugins inside a NAV block, then the actual plugin contents inside a
DIV.contentgroup-items block. In this example, the nav block is always shown,
but the items are only shown once the corresponding navigation element is
clicked. Adding this render_plugin_block makes it significantly more intuitive
to edit a child plugins content, by double-clicking its nav item in edit mode.

@mkoistinen mkoistinen changed the title Add template tag and documentation Add template tag and documentation for a new template tag render_plugin_block Nov 19, 2014
@mkoistinen mkoistinen changed the title Add template tag and documentation for a new template tag render_plugin_block Add new template tag render_plugin_block Nov 19, 2014
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a with context.push():

@mkoistinen
Copy link
Contributor Author

Thanks for the review, @ojii. Changes made.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could just return here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"explicit is better than implicit"

I'm not sure others would understand what was being returned without this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully agree, but fair enough. Really just a minor nit-pick anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, but you're the author of ClassyTags, so what is returned is obvious to you. =)

@yakky
Copy link
Member

yakky commented Nov 19, 2014

Why not making it an inclusion tag?
It would ease the manteinance of the rendered template and will also be in line with CMSEditableObject and related templatetags

@mkoistinen
Copy link
Contributor Author

@yakky the "model" here is the render_model_block tag. In the way this is used, the author of a "container" plugin will emit other markup to serve as a proxy for the change form link. In some cases it might be an icon or in others would be a "menu" item of sorts, so, I'm thinking a block tag provides the right level of flexibility, no? I'm not seeing how an inclusion tag would provide a better experience. Happy to hear/read further narrative on this though.

@yakky
Copy link
Member

yakky commented Nov 20, 2014

@mkoistinen
Copy link
Contributor Author

Ah, fair enough, @yakky, will fix soon.

@yakky yakky modified the milestone: 3.1 Nov 22, 2014
@mkoistinen
Copy link
Contributor Author

Much simpler now. Not sure why I was making it so complicated before =/ Thanks, guys!

@mkoistinen
Copy link
Contributor Author

@evildmp, @yakky, @ojii, see any reason we can't drop this into 3.0.7 too?

@evildmp
Copy link
Contributor

evildmp commented Nov 24, 2014

I see no reason why not.

@ojii
Copy link
Contributor

ojii commented Nov 24, 2014

This is a new feature, so shouldn't go into a bugfix only release (aka 3.0.7) but rather into the next release (3.1).

@evildmp
Copy link
Contributor

evildmp commented Nov 24, 2014

I don't see a reason for actively excluding it though. It's a tiny thing, and changes nothing. What difference does it make if it arrives now or later?

@mkoistinen
Copy link
Contributor Author

Was 3.0.7 meant to be "bugfix only" or "stability release"? This is tiny, stable and harmless to the rest of the system (self-standing) and I would argue does not reduce stability.

I think we should stick to our guns on this, but let's agree on what those guns are =)

@ojii
Copy link
Contributor

ojii commented Nov 24, 2014

I don't see a reason for actively excluding it though. It's a tiny thing, and changes nothing. What difference does it make if it arrives now or later?

What defines tiny/changes nothing? If we allow feature in bugfix releases, it'll become difficult to distinguish between features that are allowed and those that are not. It makes a difference because we can have a "versioning contract" with our users.

Was 3.0.7 meant to be "bugfix only" or "stability release"? This is tiny, stable and harmless to the rest of the system (self-standing) and I would argue does not reduce stability.

See above, but just a sidenote: Features like this (that only add, and do not alter things) could very nicely live in a 3rd party app until the next release (see eg staticfiles in Django).

@mkoistinen
Copy link
Contributor Author

@ojii, what you say makes sense… if you define 3.0.x as "bug-fix only". However, your interpretation effectively puts 3.0.x into maintenance, which is rather odd, because we do not yet have a usable successor (in PyPI). In my experience, a release is put into maintenance only once its successor (3.1 in this case) has been released. I'm not sure this was our intent.

I don't feel particularly strong about this tiny feature, but I'm rather more concerned now about our definitions and the fate of 3.0.x.

@ojii
Copy link
Contributor

ojii commented Nov 24, 2014

If 3.0.x aren't bugfix releases, then what are they? Or rather, what would 3.1 be? If we are fine with merging features into 3.0.x, that sounds like we'd end up with a rolling release strategy. I'm not saying that's necessarily bad, it's just different to what we historically did.

Historically we tried to follow a slightly modified semver [1]:

  1. MAJOR version: Big massive changes (only happened once really for now, let's keep it at that :D)
  2. MINOR version: Features, backwards-incompatible changes (after deprecation period)
  3. PATCH version: Bugfixes only

(The notable difference is that in MINOR version releases, backwards incompatible changes are allowed as long as they've been gone through deprecation).

[1] http://semver.org/

@mkoistinen
Copy link
Contributor Author

@ojii, I agree with you. But, I think you'll agree that, historically, CMS hasn't been doing exactly this, nor should it have been, IMHO. There are always new, smallish features added to every release I'm aware of, and certainly 3.0.1, 3.0.2, ... 3.0.6. The fact is, we're not prepared to use the semver.org versioning system pedantically as it requires more resources than we have to issue a formal release anytime there is a commit with features–OR, it means a much slower rate of progress. Although, there have been requests for this (https://plus.google.com/103026533969319599841/posts/FLQWZPgtxxb) sort of thing.

My interpretation of recent agreements was that we wanted to have a "stability release". I didn't think this precluded adding new, low risk features (of which this feature could be the poster-child for).

I'll leave this in the 3.1 branch for now. Perhaps we can/should clarify our policy in another discussion somewhere though?

@ojii
Copy link
Contributor

ojii commented Nov 24, 2014

I wasn't saying we should do strict semver, it's a bit too restrictive in my taste (we'd be at 20.1.5 by now). Also by historically I meant pre 3.0.

@yakky
Copy link
Member

yakky commented Nov 24, 2014

I think this is a suitable discussion for the technical board to discuss. (In my own view as long as there is no greater MINOR/MAJOR version, PATCH should allow a certain degree of features, yet there is no clear parameter to decide what should go in and what doesn't)

@ojii
Copy link
Contributor

ojii commented Nov 25, 2014

@yakky let's set a date!

@yakky
Copy link
Member

yakky commented Dec 8, 2014

@mkoistinen is this ready? If so, please set needs "review label"

@mkoistinen
Copy link
Contributor Author

I think so!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants