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

"Plone 5 rocks!" message should be removed from the default theme #974

Closed
hvelarde opened this issue Sep 14, 2015 · 30 comments · Fixed by #1374
Closed

"Plone 5 rocks!" message should be removed from the default theme #974

hvelarde opened this issue Sep 14, 2015 · 30 comments · Fixed by #1374

Comments

@hvelarde
Copy link
Member

According to @agnogueira this message is included in https://github.com/plone/plonetheme.barceloneta/blob/325e892db8f0f77ec37dd3ebbaa140c431197ade/plonetheme/barceloneta/theme/index.html#L36:L44

selection_004

@hvelarde hvelarde added this to the Plone 5.0 milestone Sep 14, 2015
@pbauer
Copy link
Member

pbauer commented Sep 14, 2015

It is here: https://github.com/plone/plonetheme.barceloneta/blob/master/plonetheme/barceloneta/theme/index.html#L36

It is included on the frontpage like this:

 <drop css:theme="div.principal" css:if-not-content="body.template-document_view.section-front-page" />

(see https://github.com/plone/plonetheme.barceloneta/blob/master/plonetheme/barceloneta/theme/rules.xml#L36).
Personally I like it but I understand that it can be confusing since it is not editable ttw.

@hvelarde
Copy link
Member Author

IMO, this is a potential source of problems and must be removed

@vangheem
Copy link
Member

I agree. Move the saying to the body text if we want to keep it.

@esteele
Copy link
Member

esteele commented Sep 14, 2015

My plan is to replace that with a link to the feature list.

My other complaint with the text there is that it's not translatable since it's part of index.html.I'm kind of tempted to stick another portlet manager in above the content body and use that to insert this design element.

@hvelarde
Copy link
Member Author

@esteele I though we wanted to get rid of portlets, not add more; IMO, the footer portlet manager should be deprecated as soon as Mosaic is ready.

@vangheem
Copy link
Member

Yes, please no more portlet managers :). Can't we just put it in the body html? We can put more rich html in there--it won't get stripped.

@polyester
Copy link
Member

I concur, no more portlet managers. I do very much like the idea of having a 'hero' element (which this basically is) in the default Barceloneta theme; but issues of translation could make that difficult.

In a pinch, I would probably remove it entirely, but leave it well-commented (as comments) in Barceloneta. And then write a page "How to enable a Hero element on your front page" in the docs. I volunteer do that in Munich. It would serve as a perfect example of the powers of Diazo, without posing difficulties in the initial user experience especially of non-English speakers.

Our resources are minified before putting in production; so having comments in the non-minified version are acceptable.

@esteele
Copy link
Member

esteele commented Sep 15, 2015

Fair enough. It's an element of this theme, not every Plone site.

On Sep 14, 2015, at 8:12 PM, Nathan Van Gheem notifications@github.com wrote:

Less, please no more portlet managers. Can't we just put it in the body html? We can put more rich html in there--it won't get stripped.


Reply to this email directly or view it on GitHub.

@pbauer
Copy link
Member

pbauer commented Sep 16, 2015

I changed the hero-elemt into a view @@hero that is included in the frontpage by rules.xml of barceloneta.
This way the strings are translatable, it is editable (not ZMI, later collective.jbot), it is configurable (edit the rules.xml). And it is a example how to include content using diazo in barceloneta.

@hvelarde
Copy link
Member Author

I still don't understand why we need this on the theme, never mind...

@pbauer
Copy link
Member

pbauer commented Sep 16, 2015

Because it look awesome :-)
I added a quick plone5-branch to https://github.com/collective/collective.behavior.banner that makes banners look exactly like the hero-unit. This way it is easily configurable and editable ttw. But that is not a solution for the core. Including a view in diazo was the best solution we came up with (and we discussed quite a lot of solutions).

@hvelarde
Copy link
Member Author

if we already have add-ons for that I see less and less justification to add this into the core; sometimes I don't understand why we have to complicate things instead of simplify them.

but, as I said before, never mind...

@thet
Copy link
Member

thet commented Sep 16, 2015

I was quite confused about that myself, when I came up with the idea to remove it some time ago. I thought it was a viewlet, activated directly on the context. but it isn't. I was OK with removing the page makes it go away. I don't like the current solution either.

Why not just add this as HTML directly to the body text? Maybe even create a tinyMCE custom class, which adds a div with class hero?

@hvelarde
Copy link
Member Author

@pbauer don't forget to document this, please; we don't want more Easter eggs.

@pbauer
Copy link
Member

pbauer commented Sep 16, 2015

@hvelarde Will do.
@thet Can you come up with readable diazo-rules to do that? The current solution is in no way perfect.

@djay
Copy link
Member

djay commented Sep 16, 2015 via email

@esteele
Copy link
Member

esteele commented Sep 16, 2015

Putting it directly into the html doesn't allow us to translate the text. This implementation also makes a nice example of side-loading content in a Diazo theme.

On Sep 16, 2015, at 4:51 PM, Johannes Raggam notifications@github.com wrote:

I was quite confused about that myself, when I came up with the idea to remove it some time ago. I thought it was a viewlet, activated directly on the context. but it isn't. I was OK with removing the page makes it go away. I don't like the current solution either.

Why not just add this as HTML directly to the body text? Maybe even create a tinyMCE custom class, which adds a div with class hero?


Reply to this email directly or view it on GitHub.

@davilima6
Copy link
Member

I agree that with proper documentation it's a nice to have example, maybe we could grow it a bit to include sth dynamic, only to better legitimate a Python view.

However I see the risk of increasing our Zope request count and page loading time for sth that could easily be in docs only. IMO we should aim for the smallest SLOC in the otherwise alien rules.xml.

One sidenote: "Plone rocks" is awesome but doesn't have a consistent translation across languages. Each will probably have distinct variations, requiring creativity from translators and kind of setting loose the global message tone.

@fulv
Copy link
Member

fulv commented Sep 16, 2015

Just my 2c: instead of putting the view in CMFPlone, add
plone.app.themingplugins to barceloneta, and put it there. After all,
bobtemplates.plone uses p.a.themingplugins, and bob is the "official"
story, so we would be consistent.

On Wed, Sep 16, 2015 at 11:42 AM Davi Lima notifications@github.com wrote:

I agree that with proper documentation it's a nice to have example, maybe
we could grow it a bit to include sth dynamic, only to better legitimate a
Python view.

However I see the risk of increasing our Zope request count and page
loading time for sth that could easily be in docs only. IMO we should aim
for the smallest SLOC in the otherwise alien rules.xml.


Reply to this email directly or view it on GitHub
#974 (comment)
.

@davilima6
Copy link
Member

If we go for a view I agree with you, @fulv, better prefer the "bob story". However I believe p.a.themingplugins wasn't included in core also because of performance reasoning (quicker core zopestart and pageloads).

@djay
Copy link
Member

djay commented Sep 16, 2015

You can translate it at the time you insert it, same as you do the body
text, in fact make it part of the bodytext as suggested. Teaching them to
use a custom class and diazo to transform content they can edit easily is a
far more powerful concept that side loading static views. You only ever
want to use side loading for something dynamic like a last updated date
(and even then I'd probably use a portlet and diazo). In fact our preferred
method of a doing something like this is a top portlet manager via content
well portlets, and a hero portlet style via portlet style plugin. Its a
shame neither of those got built into the core.

What you have given them is content that is almost impossible for them to
manage which seems counter intuitive for a CMS.

@pbauer
Copy link
Member

pbauer commented Sep 18, 2015

Unless some comes up with a better solution (and ideally a PR) we will stick with what we have now. Any solution must meet the following requirements:

  • no additional dependencies
  • translateable
  • configurable ttw
  • no xslt in rules.xml
  • no additional content or portlet(manager)

The current solution is by no means ideal. Personally I like the plone.app.themingplugins-approach and we could easily move to that when it makes it in the core. But the perfect future solution will probably be a richtext-tile added to the front-page with p.a.mosaic.

@djay
Copy link
Member

djay commented Sep 18, 2015

Why are you dismissing the idea of an above content portlet manager so
quickly? To me its as useful as the footer portlet manager which is now
built in and would remove our use of yet another plugin as we use it for
many of our sites. It also fits well with the future direction of plone
using mosaic as that allows tiles anywhere on a page layout. Not just sides
and footer.
IMO putting "plone rocks" into the theme via themingplugins or similar is
not right. Its content and should be editable by content editors not
themers.

On Fri, 18 Sep 2015 12:19 pm Philip Bauer notifications@github.com wrote:

Unless some comes up with a better solution (and ideally a PR) we will
stick with what we have now. Any solution must meet the following
requirements:

  • no additional dependencies
  • translateable
  • configurable ttw
  • no xslt in rules.xml
  • no additional content or portlet(manager) The current solution is by
    no means ideal. Personally I like the plone.app.themingplugins-approach and
    we could easily move to that when it makes it in the core. But the perfect
    future solution will probably be a richtext-tile added to the front-page
    with p.a.mosaic.


Reply to this email directly or view it on GitHub
#974 (comment)
.

@thet
Copy link
Member

thet commented Sep 18, 2015

+1 @pbauer, looks like something where we cannot easily come up with a really good solution.

+1 @djay, since we obviously already use portlets a lot, we need some migration code to anything better (mosaic?) anyways. so another portlet manager wouldn't hurt too much. except we would update docs for it and portlets can be a big development pain. i personally like the idea of ContentWellPortlets, which allow quite flexible page compositions.

@sverbois
Copy link

+1 @djay, I add top page and bottom page portlet managers in all my sites for years

gyst added a commit that referenced this issue Feb 10, 2016
gyst added a commit to plone/plonetheme.barceloneta that referenced this issue Feb 10, 2016
@gyst
Copy link
Member

gyst commented Feb 10, 2016

I've pushed branches to manage the hero from content, but cannot find how to add a 'hero' class to TinyMCE.

@gyst gyst reopened this Feb 10, 2016
@gyst
Copy link
Member

gyst commented Feb 10, 2016

Oh and #992 should be reverted. Or at least the hero view removed.

@djay
Copy link
Member

djay commented Feb 10, 2016

@gyst #492

@davilima6
Copy link
Member

@gyst: You can add a hero template: http://training.plone.org/5/theming/tinymce-templates.html Wouldn't that be more suitable?

@gyst
Copy link
Member

gyst commented Feb 11, 2016

I've issued a set of two PRs that improves on the old situation by moving the hero to content, where it can be easily removed by editors.

Adding new hero elements is something else and still needs to be done. @davilima6 the tinymce templates you mention look like a bit of overkill to be. I've included the necessary wrapper classes in Barceloneta. Al one needs to do is have a <div class="hero"> ... </div>.

gyst added a commit that referenced this issue Feb 22, 2016
gyst added a commit to plone/plonetheme.barceloneta that referenced this issue Feb 22, 2016
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Feb 22, 2016
Branch: refs/heads/master
Date: 2016-02-22T16:52:44+01:00
Author: Guido A.J. Stevens (gyst) <guido.stevens@cosent.nl>
Commit: plone/plonetheme.barceloneta@0b849a1

move hero to content
fixes plone/Products.CMFPlone#974
requires plone/Products.CMFPlone#1374

Files changed:
M CHANGES.rst
M plonetheme/barceloneta/theme/index.html
M plonetheme/barceloneta/theme/rules.xml
Repository: plonetheme.barceloneta
Branch: refs/heads/master
Date: 2016-02-22T17:08:21+01:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/plonetheme.barceloneta@d3838e1

Merge pull request #83 from plone/hero_in_content

move hero to content, refs https://github.com/plone/Products.CMFPlone…

Files changed:
M CHANGES.rst
M plonetheme/barceloneta/theme/index.html
M plonetheme/barceloneta/theme/rules.xml
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Feb 22, 2016
Branch: refs/heads/master
Date: 2016-02-22T15:36:58+01:00
Author: Guido A.J. Stevens (gyst) <guido.stevens@cosent.nl>
Commit: plone/Products.CMFPlone@5ced858

move hero to content
fixes plone/Products.CMFPlone#974
requires plone/plonetheme.barceloneta#83

Files changed:
M CHANGES.rst
M Products/CMFPlone/browser/configure.zcml
M Products/CMFPlone/browser/templates/plone-frontpage.pt
D Products/CMFPlone/browser/templates/hero.pt
Repository: Products.CMFPlone
Branch: refs/heads/master
Date: 2016-02-22T17:08:28+01:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/Products.CMFPlone@a74391d

Merge pull request #1374 from plone/hero_in_content

move hero to content

Files changed:
M CHANGES.rst
M Products/CMFPlone/browser/configure.zcml
M Products/CMFPlone/browser/templates/plone-frontpage.pt
D Products/CMFPlone/browser/templates/hero.pt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.