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

Select multiple class in cover configuration #779

Merged
merged 7 commits into from
Apr 26, 2018
Merged

Conversation

rodfersou
Copy link
Member

WIP

</option>
</tal:classes>
</select>
<input class="cssclasswidget" type="button" value="Select options ▼"
Copy link
Member

Choose a reason for hiding this comment

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

value needs to be internationalized.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -731,3 +731,45 @@ input[type='submit'].btn::-moz-focus-inner
{
cursor: move;
}
body .cssclasswidget
Copy link
Member

Choose a reason for hiding this comment

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

how are we going to deal with intermediate caching servers and this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

the simplest solution in this case is to rename the resource name appending the modify date.

@@ -1,4 +1,112 @@
(function($) {
var CSSClassWidget = (function() {
Copy link
Member

Choose a reason for hiding this comment

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

how are we going to deal with intermediate caching servers and this change?

Copy link
Member Author

@rodfersou rodfersou Apr 25, 2018

Choose a reason for hiding this comment

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

this needs an upgrade step to cook JS files since this file is into jsregistry

<input class="cssclasswidget" type="button" value="Select options ▼"
Copy link
Member

Choose a reason for hiding this comment

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

value needs to be internationalized.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

items = [
{
key: value
for key, value in item.iteritems()
Copy link
Member

Choose a reason for hiding this comment

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

iteritems() works on Python 2 only; you need to import it from six; check http://python-future.org/compatible_idioms.html#iterating-through-dict-keys-values-items

CHANGES.rst Outdated
@@ -6,6 +6,9 @@ There's a frood who really knows where his towel is.
1.6b6 (unreleased)
^^^^^^^^^^^^^^^^^^

- Select multiple CSS class in cover configuration.
Copy link
Member

Choose a reason for hiding this comment

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

"Allow selection of multiple CSS classes in tile configuration."

@@ -51,13 +51,9 @@
<div id="slider"></div>
</div>
<div id="class-chooser" title="Choose Class">
Copy link
Member

Choose a reason for hiding this comment

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

"Choose classes"; title needs to be internationalized.

r = i % 4 * 64
g = i % 8 * 32
b = i % 16 * 16
r = i % 4 * 64 # noqa: S001
Copy link
Member

Choose a reason for hiding this comment

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

don't include this as this bug is already fixed: gforcada/flake8-pep3101#16

what version of flake8-pep3101 are you using?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know how to check this out

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.

2 participants