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

Requirement for custom modernizr build adds maintenance work #5

Open
cherrydev opened this issue Nov 26, 2014 · 0 comments
Open

Requirement for custom modernizr build adds maintenance work #5

cherrydev opened this issue Nov 26, 2014 · 0 comments

Comments

@cherrydev
Copy link

Why

I was pretty surprised to discover that to use this otherwise well-thought-out library, I needed to patch modernizer, or else it would not work in Firefox. In looking into it further, it appears to be completely unnecessary and could be remedied with a js file less than a few hundred bytes. While I'm sure the following portion describing an implementation is probably obvious to you, I mostly wanted to convince you that this is a good idea. While I can understand your desire to keep the requirements for flexgrid as small as possible, I believe that you have inadvertently gone the opposite direction by requiring a customized version of Modernizer:

  1. This can potentially break other libraries that rely on the core modernizer tests working the standard way.
  2. Any time in the future that a developer wishes to upgrade modernizer, they will need to have a portion of their workflow dedicated to patching it for flexgrid.
  3. You already require 1 (or 2) javascript files in order for flexgrid to work, so it's not as if it's some sort of fundamental shift in the design philosophy of the library.

How

First of all, starting from a stock non-customized modernizer 2.8.3 release, the following changes appear to be unnecessary, as it's already implemented this way:

hgroup,nav → hgroup,main,nav
hgroup mark → hgroup main mark

Next, by changing flexboxlegacy → webkitbox and boxDirection → WebkitBoxFlex you've just completely removed one test and replaced it with a totally different one. I can't see how the old test interferes at all. The required javascript would be:

Modernizer.addTest('webkitbox', function() { return Modernizer.testAllProps('WebkitBoxFlex'); });

Finally, as for the flexWrap → flexDirection change, there is currently no supported way in modernizer to replace a core test, but the following javascript line would add a new test you could use instead:

Modernizer.addtest('flexboxflexgrid', function() { return Modernizer.testAllProps('flexDirection'); });

Make it Optional

It would already be possible to make flexgrid work without changing the css file and adding a small javascript file, if it were not for the fact that flexgrid relies on the different behaviour of the exiting test 'flexbox'. If you made your patched Modernizr add the new 'flexboxflexgrid' test and changed the css to test for it, it would allow for users to make use of flexbox without having to use a patched version of either modernizer or flexbox.
At that point, you could give users the option of either:

  1. Using your very minimal, patched version of Modernizer
  2. Using a standard version of Modernizer that has 'addTest' enabled, plus a minimal extra javascript file.
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

No branches or pull requests

1 participant