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

Remove Old Foundation Files #251

Merged
merged 2 commits into from
Mar 11, 2018
Merged

Conversation

jessedoyle
Copy link
Contributor

Hi All,

I think this gem may be including some utility JS code from older versions of Foundation.

In this diff, you can see that the foundation.util.timerAndImageLoader was split into 2 separate JS files (timer.js and imageLoader.js).

The most current version of this gem appears to be loading both files here.

It also appears that the same scenario above played out with foundation.responsiveAccordionTabs.js (newer) and foundation.zf.responsiveAccordionTabs.js.

I believe this was noted in this issue upstream: foundation/foundation-sites#9697.

I took the liberty of updating the version in this PR as a bugfix - please let me know if you wish to do this yourself.

Thanks!

* Foundation 6.4.1 refactored the `foundation.util.timerAndImageLoader`
  into separate files.
* The imageLoader and timer utilities are already being loaded in
  `foundation.js`, so the older `timerAndImageLoader` can safely
  be removed from this repo.
* Remove the out-of-date `foundation.zf.responsiveAccordionTabs.js`
  utility under the same rationale as above.
* A separate PR will be made upstream against `foundation-sites` to
  remove these out of date files from the `dist` directory`.
* Bump version to indicate that this is a bugfix.

SEE: foundation/foundation-sites@0730b7b#diff-11ba5437268f42332a0ff3914daf8edd
SEE: foundation/foundation-sites#9697
@jessedoyle
Copy link
Contributor Author

jessedoyle commented Feb 9, 2018

@rafibomb @kball - I just want to bump this as it appears that a number of people are running into this issue.

In our case, the old JS files are appending a parameter that is blocked by our security appliance.

@darkodosenovic
Copy link

👍 +1

Versions should be bump only in package releases
Copy link
Contributor

@ncoden ncoden left a comment

Choose a reason for hiding this comment

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

Thanks @jessedoyle. LGTM 👍
I removed the version bump as this is something only the package maintainer should do when a new version is released.

@ncoden ncoden merged commit 4f33648 into foundation:master Mar 11, 2018
@jessedoyle jessedoyle deleted the foundation-6.4.1 branch March 11, 2018 21:25
@jessedoyle
Copy link
Contributor Author

Awesome thanks @ncoden! Sorry about the version bump!

@ncoden
Copy link
Contributor

ncoden commented Mar 13, 2018

No problem @jessedoyle. Thanks for your contributions.

ncoden added a commit to ncoden/foundation-rails that referenced this pull request Mar 13, 2018
### Major changes:
* Upgrade to Foundation `v6.4.3` (foundation#254, @chrisfinne)
* Add Autoprefixer (foundation#255, @bassjobsen)

### Bug fixes:
* Fix issue with smoothScroll import order (foundation#236 @MicahBrown)
* Add missing SCSS components to template (foundation#237 @patricklindsay)
* Prevent Sass depreciation (foundation#252, @michsch)

### Cleaning:
* Remove old Foundation files (foundation#251 @jessedoyle)

### See also:
* Foundation 6.4.3 release notes:
  https://github.com/zurb/foundation-sites/releases/tag/v6.4.3
* Foundation 6.4.2 release notes:
  https://github.com/zurb/foundation-sites/releases/tag/v6.4.2
jessedoyle added a commit to amaabca/ama_layout that referenced this pull request Apr 11, 2018
* Remove the `util.timerAndImageLoader` reference in our custom
  foundation javascript includes because it is no longer present
  upstream.
* Bump `foundation-rails` version to require `6.4.3.0` or higher.
* This will bump our Foundation version from `6.4.1` to `6.4.3`.
  The changes that requires migration between these are to the
  xy-grid, which we do not currently use.

SEE: https://github.com/zurb/foundation-sites/releases/tag/v6.4.2
SEE: foundation/foundation-rails#251
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.

3 participants