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

Add print plugin, use npm package via yarn #51

Closed
wants to merge 4 commits into from
Closed

Conversation

thet
Copy link
Member

@thet thet commented Jan 15, 2018

  • Add the print plugin per default, so that images are loaded before the page is going to be printed.
  • Use the npm package and yarn to install lazysizes for a defined way to update / add resources. Via .yarnrc a custom location instead node_modules is defined.

Fixes #50

Add the print plugin by default.
Use ``yarn`` to manage JavaScript dependencies and upgrade resource paths to new location.
@thet thet requested a review from hvelarde January 15, 2018 01:47
@@ -47,7 +47,7 @@ def setUp(self):

def test_upgrade_to_2_registrations(self):
version = self.setup.getLastVersionForProfile(self.profile_id)[0]
self.assertGreaterEqual(version, self.to_version)
self.assertGreaterEqual(int(version), int(self.to_version))
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 string in int cast is necessary. The tests passed before this change for metadata version 9 only by the way strings are compared in python. See below:

Comparison of "10" with other numbers-as-strings:

>>> "10" >= "0"
True
>>> "10" >= "1"
True
>>> "10" >= "2"
False
>>> "10" >= "3"
False
>>> "10" >= "4"
False
>>> "10" >= "5"
False
>>> "10" >= "6"
False
>>> "10" >= "7"
False
>>> "10" >= "8"
False
>>> "10" >= "9"
False
>>> "10" >= "10"
True

Comparison of "9" with other numbers-as-strings:

>>> "9" >= "0"
True
>>> "9" >= "1"
True
>>> "9" >= "2"
True
>>> "9" >= "3"
True
>>> "9" >= "4"
True
>>> "9" >= "5"
True
>>> "9" >= "6"
True
>>> "9" >= "7"
True
>>> "9" >= "8"
True
>>> "9" >= "9"
True
>>> "9" >= "10"
True

@thet
Copy link
Member Author

thet commented Jan 17, 2018

travis tests are failing in the moment because of the Plone 4.3 integration.

I don't like having the jsregistry.xml GS profile file around for Plone 5.x - this is registering the resources into the plone-legacy bundle - along with the registration of the lazysizes bundle. That way we end up with a double reigstration of these resources.
I'd actually vote to remove the Plone 4.3 support. Or doing the registration with separate 4.3 and 5.x profiles, as I did for my initial Plone 5.x support PR.

Any comments?

@rodfersou
Copy link
Member

please, use sc.recipe.staticresources https://github.com/simplesconsultoria/sc.recipe.staticresources and webpack for this (we are migrating all of our addons to use it)

Copy link
Member

@hvelarde hvelarde left a comment

Choose a reason for hiding this comment

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

after merging #53 this is no longer the way to fix this issue.

@hvelarde
Copy link
Member

hvelarde commented Sep 5, 2018

superseded by #59

@hvelarde hvelarde closed this Sep 5, 2018
@hvelarde hvelarde deleted the thet-yarn-print branch September 5, 2018 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants