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

Default tile class #262

Closed
agnogueira opened this issue Aug 21, 2013 · 5 comments · Fixed by #267
Closed

Default tile class #262

agnogueira opened this issue Aug 21, 2013 · 5 comments · Fixed by #267
Assignees
Labels
Milestone

Comments

@agnogueira
Copy link
Contributor

The initial class for tiles is "empty" and add a strange code to HTML:

<div class="tile --NOVALUE--">

We should change this to "Default" and add a better style for that, like this:

<div class="tile tile-default">
@ghost ghost assigned jpgimenez and marcosfromero Aug 21, 2013
@marcosfromero
Copy link
Contributor

Can't reproduce.
Instead, I get this strange class attributes for every kind of tile:

{'order':​ u'0', 'visibility':​ u'on'}​

>>> $('div.tile')
[
<div data-tile=​"@@collective.cover.banner/​e612e18df292495d940ff21fc9b5890d" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"e612e18df292495d940ff21fc9b5890d">​…​</div>​
, 
<div data-tile=​"@@collective.cover.basic/​df06ea7fbb1e4327badf17c9d522cf52" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"df06ea7fbb1e4327badf17c9d522cf52">​…​</div>​
, 
<div data-tile=​"@@collective.cover.carousel/​b2b56d80efdb4648acb0ee13acad83b0" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"b2b56d80efdb4648acb0ee13acad83b0">​…​</div>​
, 
<div data-tile=​"@@collective.cover.collection/​fd97968459d24668ab46d558ebcfe99a" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"fd97968459d24668ab46d558ebcfe99a">​…​</div>​
, 
<div data-tile=​"@@collective.cover.contentbody/​aa26cf5a807a4f758b2705c9a078c1d2" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"aa26cf5a807a4f758b2705c9a078c1d2">​…​</div>​
, 
<div data-tile=​"@@collective.cover.embed/​f9051b770e584625aa8a1ce96cd9789f" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"f9051b770e584625aa8a1ce96cd9789f">​…​</div>​
, 
<div data-tile=​"@@collective.cover.file/​c102442f13c84022937a3c4522094c92" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"c102442f13c84022937a3c4522094c92">​…​</div>​
, 
<div data-tile=​"@@collective.cover.list/​d9e45e92577147c297b004cde13940d8" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"d9e45e92577147c297b004cde13940d8">​…​</div>​
, 
<div data-tile=​"@@collective.cover.richtext/​f96feb77529444cbaf3d32c93f0b5d1a" class=​"tile {'order':​ u'0', 'visibility':​ u'on'}​ ui-droppable" id=​"f96feb77529444cbaf3d32c93f0b5d1a">​…​</div>​
]

@hvelarde
Copy link
Member

well, that's worst then and we have to fix it...

@hvelarde hvelarde reopened this Aug 26, 2013
@marcosfromero
Copy link
Contributor

These strange values ({'order':​ u'0', 'visibility':​ u'on'}​) are returned by the ITilesConfigurationScreen adapter: when no value is set for a certain configuration option (i.e. field), like css_class, the adapter sets these default values and returns them:
https://github.com/collective/collective.cover/blob/master/src/collective/cover/tiles/configuration.py#L112

This behavior occurs only if configuration options where not saved.

If configuration was set and "No value" was chosen for CSS Class field, then the aforementioned --NOVALUE-- class name is used.

Changing this to tile-default but consider opening a new issue.

@hvelarde
Copy link
Member

I think this has to be implemented here: https://github.com/collective/collective.cover/blob/master/src/collective/cover/tiles/configuration.py#L93

don't forget to improve testing on this also.

@hvelarde
Copy link
Member

@marcosfromero according to @agnogueira is better to declare the field as required and provide an upgrade step for all tiles in all covers on the site; please modify your pull request according to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants