ZestfulNation / vue-hotel-datepicker

A VueJS v2 responsive date range picker. Displays the number of nights selected and allow several useful options like custom check-in/check-out rules, localization support and more.
https://zestfulnation.github.io/vue-hotel-datepicker/
MIT License
840 stars 220 forks source link

Complete rewrite in fork #192

Closed matiasperrone closed 4 years ago

matiasperrone commented 4 years ago

@joffreyBerrier said:

Hi I have created a fork of the repo and i have do some changes

https://github.com/joffreyBerrier/vue-hotel-datepicker/projects/1 https://github.com/joffreyBerrier/vue-hotel-datepicker

@cjlaborde said:

I didn't like that you removed the original design, it doesn't look good now, I preferred the green look since it matched my project color theme. As well-liked a lot the shadowed effect on Selecting date with white background. Don't like the gray at all preferred the lighter green. Why the issue tab not available in your fork? Can you add the original design back? and enable the issue tab? I did like a lot the changes you made like separated the CSS and removed pug with html.

@matiasperrone (colaborator) said:

All these changes are available in v3 did you try it?

@cjlaborde said:

Does v3 has these features that Joffrey implemented? Use vue-cli Remove the pug html, use html Remove some useless dependencies Rewrite code Add eslint airbnb + prettier Remove useless condition like empty if or else Add return for all computed Add a bind key on v-for Rewrite jest test Remove v-html / v-text use {{ }} Add emit when clearSelection Remove querySelector, use refs Fix bug: impossible to open the calendar when clearselection is trigger Allows to have half a day, if you have check in at noon and checkout before noon

Notice that v3 still uses pug instead of HTML As well does it has separated css you can access like this? import 'vue-hotel-datepicker2/dist/vueHotelDatepicker2.css';

matiasperrone commented 4 years ago

@cjlaborde

matiasperrone commented 4 years ago

@joffreyBerrier

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

joffreyBerrier commented 4 years ago

Hi @matiasperrone

Why did you create this issue what are you waiting for?

You want me to create a 4.0.0 branch with my own code?

joffreyBerrier commented 4 years ago

Sorry I don't saw your question @cjlaborde.

I remove some useless CSS, this repo contains lot of dead code.

I put my own style because I used this repo at my work. The new style correspond to the website.

But you’re talking about CSS, this is something that you can change easily. I haved focus on the JS code.

You are soon have the possibility to custom CSS with variable

matiasperrone commented 4 years ago

Seems apropiate!! exactly what I'd do.

Hi @matiasperrone

Why did you create this issue what are you waiting for?

You want me to create a 4.0.0 branch with my own code?

matiasperrone commented 4 years ago

@joffreyBerrier Do you want to help me maintain the repo?

joffreyBerrier commented 4 years ago

Yes @matiasperrone, but is it possible to keep my code?

I modified and improved a lot of code and added new features, I can't lose all of that because I use my fork on a production site which needs all my new features

matiasperrone commented 4 years ago

Sure, I think that we can make a merge of the repositories. You can, if you want provide a PR, what do you think?

joffreyBerrier commented 4 years ago

Yes sure @matiasperrone , I have a few things to do before that ! Rest in touch !

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

matiasperrone commented 4 years ago

@joffreyBerrier Any news?

matiasperrone commented 4 years ago

The changes were incorporated in the v4.0.0-dev https://github.com/krystalcampioni/vue-hotel-datepicker/releases/tag/v4.0.0-dev

matiasperrone commented 4 years ago

Comments?

mariusa commented 4 years ago

Hi! Excited to try it out!

Please update the README (has old info), the demo & publish v4 to npm.

superbiche commented 4 years ago

Would love to see a NPM published version for v4 dev too

matiasperrone commented 4 years ago

I am having some issues with the build 😅

superbiche commented 4 years ago

@matiasperrone maybe we can help?

matiasperrone commented 4 years ago

Can you download and try to run "build". I tried with pnpm (is the one that I always use) and it didn't work, I did not have the time to continue so...

superbiche commented 4 years ago

There you are: https://github.com/krystalcampioni/vue-hotel-datepicker/pull/220

matiasperrone commented 4 years ago

Thanks!

superbiche commented 4 years ago

Any ETA for the merge + package release ? I can also help. Feel free to add me as a contributor, this package is the only one that answers correctly one of my biggest client's requirements, so I'll probably manage to free some time to help maintaining/enhancing it

matiasperrone commented 4 years ago

I uploaded to npm vue-hotel-datepicker@4.0.0-beta.4

mariusa commented 4 years ago

The build doesn't work, when running vue3 I get GET https://localhost:3000/@modules/vue-hotel-datepicker/dist/vueHotelDatepicker.common.js net::ERR_ABORTED 404 (Not Found)

package.json.txt

App.vue.txt

The whole project, if needed, ready to run (rename .zip to .tgz) v3-2020-09-01.zip

superbiche commented 4 years ago

The build is still wrong: the package.json has "main": "dist/vueHotelDatepicker.common.js", but there's no such file when using vue-hotel-datepicker@4.0.0-beta.4. image

Also when using with Nuxt, I get a window is not defined error, forcing me to use <client-only> to wrap the component.

Seems the v4 complete rewrite has a lot of issues.

EDIT : when using node_modules/vue-hotel-datepicker/src/components/DatePicker/index.vue to import the source, there's another error caused by the ~/ imports :

// components/DatePicker/index.vue
import Day from '~/components/Day.vue'
import DateInput from '~/components/DateInput.vue'
import Helpers from '~/helpers'

These should be relative imports:

import Day from '../Day.vue'
import DateInput from '../DateInput.vue'
import Helpers from '../../helpers'
superbiche commented 4 years ago

https://github.com/krystalcampioni/vue-hotel-datepicker/pull/222

superbiche commented 4 years ago

Also seems you've used pnpm build instead of pnpm build-package

mariusa commented 4 years ago

Thanks @superbiche !

@matiasperrone please do a simple test after publishing to npm, to ensure your package works. You can use the project I uploaded, just run

npm install 
npm run dev
superbiche commented 4 years ago

Added a few commits to fix issues & docs regarding i18n. I guess there's more to fix, will keep you posted (and update the PR comments accordingly).

superbiche commented 4 years ago

@joffreyBerrier there are a lot of new, undocumented props you added. Could you make a PR with the updated docs please?

matiasperrone commented 4 years ago

I rebuilded the package to beta 6, and yes there are some issues. I do not have too much free time in my hands so I will be delivering fast builds, but maybe we can improve the testing, so I don't have to test too much, and you are more that welcome to help in any way you can.

Please realize that I do this in my own time. Be patient.

matiasperrone commented 4 years ago

@superbiche Michel check the beta 6. I hope it works!

mariusa commented 4 years ago

Ok, now there are no errors, but nothing shows where the datepickers should be. Screenshot from 2020-09-03 23-38-41

so I don't have to test too much,

I'll help with the detailed testing. Just asking that you please do a basic check of the package you publish (use the project I attached, or simply init a new project). Do the datepickers show? That's all. This happened on v3 as well (which I can't use due to regressions closed as fixed in v4).

I appreciate your time on this project. I never encountered though a broken npm module which the author didn't fix himself right after publishing (because he realized it was broken).

superbiche commented 4 years ago

@mariusa my PR fixes all issues so once the 2 discussions we're having with @matiasperrone are resolved, I guess he can publish a new build (using my fork I can render and use the datepicker).

@matiasperrone I'm aware builds and fixes of this package rely heavily on your free time, that's why I'm also offering to help as a co-maintainer. I have an important client in the hotel business for whom we're developing apps heavily using this package, and I absolutely don't want to fork it and create yet another fork, I'd rather focus my efforts on this one, but if it depends only on your availability (and if you're not available a lot these times,which happens) this can be a pain point for me and others.

Let me know what you think. I'm also eager to add some testing - at least for build + some Jest snapshots to make sure the layout isn't broken. But there's some work to fix the remaining bugs, update the docs according to the new props, etc... before we can do this ;)

matiasperrone commented 4 years ago

@superbiche I agree, the more the merrier! I am not the "owner" so you have to apply at #67 and ask to @krystalcampioni, she is the owner 👩‍💻 and she has the power 😁, anyway, I wrote her an email, so she can add you as a "Collaborator".

@mariusa sorry about that, can you provide with a more clear example? npm run dev is not a script in this package.

I am not longer maintain the v3 version due to the fact that it is old, and most of the "bugs" are very old (before I became a collaborator) and most of those are already solved in this new version.

superbiche commented 4 years ago

@matiasperrone thanks a lot for contacting the owner already! That's great news, I already started working on my fork and cleaning up some stuff in the v4 dev version, will publish a PR later this week with a few enhancements, mostly CSS related.

Anyway, thanks a lot and see you around soon

matiasperrone commented 4 years ago

Sure thanks!

matiasperrone commented 4 years ago

Ok. Solved (beta 8). But the problem remains when build the package with npm instead of pnpm

matiasperrone commented 4 years ago

Not to the users but to us

matiasperrone commented 4 years ago

@mariusa there you go! check out the usage.

mariusa commented 4 years ago

Thanks!

It still doesn't show for me. Using vue v3, since #213 was closed as done. I just created a vite project https://github.com/vitejs/vite and added "vue-hotel-datepicker": "4.0.0-beta.8"

If vue3 support isn't done, please re-open #213 and I'll test with vue2 after @superbiche 's upcoming PR is merged.

$ npm run dev 

> v3@0.0.0 dev /home/marius/work/v3
> vite --debug --https

vite v1.0.0-rc.4
  vite:config env mode: development +0ms
  vite:config env: {} +4ms
  vite:config config resolved in 29ms +3ms
  vite:resolve (node_module entry) vue -> dist/vue.runtime.esm-bundler.js +0ms
  vite:optimize optimizing vue (imports sub dependencies) +0ms
  vite:resolve (node_module entry) vue-hotel-datepicker -> dist/vueHotelDatepicker.common.js +2ms
  vite:optimize optimizing vue-hotel-datepicker (no exports, likely commonjs) +7ms
[vite] Optimizable dependencies detected:
vue, vue-hotel-datepicker
Pre-bundling them to speed up dev server page load...
(this will be run only when your dependencies have changed)

  Dev server running at:
  > Local:    https://localhost:3000/
  > Network:  https://192.168.100.4:3000/

  vite:server server ready in 2729ms. +0ms
  vite:hmr         / imports /src/main.js +0ms
  vite:rewrite (skipped) / +0ms
  vite:rewrite (skipped) /vite/client +100ms
  vite:rewrite /src/main.js: rewriting +1ms
  vite:rewrite     "vue" --> "/@modules/vue.js" +2ms
  vite:hmr         /src/main.js imports /@modules/vue.js +104ms
  vite:rewrite     "./App.vue" --> "/src/App.vue" +1ms
  vite:hmr         /src/main.js imports /src/App.vue +1ms
  vite:rewrite     "./index.css" --> "/src/index.css?import" +0ms
  vite:hmr         /src/main.js imports /src/index.css +0ms
  vite:resolve (optimized) vue.js -> node_modules/.vite_opt_cache/vue.js +0ms
  vite:rewrite /@modules/vue.js: no imports found. +33ms
  vite:sfc /home/marius/work/v3/src/App.vue parsed in 14ms. +0ms
  vite:rewrite /src/App.vue: rewriting +20ms
  vite:rewrite     "./components/HelloWorld.vue" --> "/src/components/HelloWorld.vue" +1ms
  vite:hmr         /src/App.vue imports /src/components/HelloWorld.vue +54ms
  vite:rewrite     "vue-hotel-datepicker" --> "/@modules/vue-hotel-datepicker.js" +0ms
  vite:hmr         /src/App.vue imports /@modules/vue-hotel-datepicker.js +0ms
  vite:hmr ws client connected +7ms
  vite:resolve (optimized) vue-hotel-datepicker.js -> node_modules/.vite_opt_cache/vue-hotel-datepicker.js +46ms
  vite:sfc /home/marius/work/v3/src/components/HelloWorld.vue parsed in 4ms. +24ms
  vite:rewrite /src/components/HelloWorld.vue: rewriting +20ms
  vite:rewrite     nothing needs rewriting. +0ms
  vite:rewrite /@modules/vue-hotel-datepicker.js: no imports found. +7ms
  vite:sfc /home/marius/work/v3/src/components/HelloWorld.vue parse cache hit +11ms
  vite:sfc /src/components/HelloWorld.vue template compiled in 32ms. +32ms
  vite:rewrite /src/components/HelloWorld.vue?type=template: rewriting +36ms
  vite:rewrite     "vue.js" --> "/@modules/vue.js" +0ms
  vite:hmr         /src/components/HelloWorld.vue?type=template imports /@modules/vue.js +56ms
  vite:sfc /home/marius/work/v3/src/App.vue parse cache hit +2ms
  vite:sfc /src/App.vue template compiled in 8ms. +8ms
  vite:rewrite /src/App.vue?type=template: rewriting +9ms
  vite:rewrite     "vue.js" --> "/@modules/vue.js" +0ms
  vite:hmr         /src/App.vue?type=template imports /@modules/vue.js +9ms
  vite:rewrite (skipped) /src/index.css?import +2ms
  vite:rewrite (skipped) /src/assets/logo.png +61ms
  vite:rewrite /: serving from cache +0ms
  vite:rewrite (skipped) / +6s
  vite:rewrite (cached) /src/main.js +193ms
  vite:rewrite (skipped) /vite/client +3ms
  vite:resolve (cached) vue.js -> node_modules/.vite_opt_cache/vue.js +6s
  vite:rewrite (cached) /@modules/vue.js +13ms
  vite:sfc /home/marius/work/v3/src/App.vue parse cache hit +6s
  vite:rewrite (cached) /src/App.vue +4ms
  vite:rewrite (skipped) /src/index.css?import +9ms
  vite:sfc /home/marius/work/v3/src/components/HelloWorld.vue parse cache hit +13ms
  vite:rewrite (cached) /src/components/HelloWorld.vue +5ms
  vite:resolve (cached) vue-hotel-datepicker.js -> node_modules/.vite_opt_cache/vue-hotel-datepicker.js +21ms
  vite:rewrite (cached) /@modules/vue-hotel-datepicker.js +2ms
  vite:sfc /home/marius/work/v3/src/App.vue parse cache hit +7ms
  vite:sfc /src/App.vue template cache hit +0ms
  vite:rewrite (cached) /src/App.vue?type=template +4ms
  vite:sfc /home/marius/work/v3/src/components/HelloWorld.vue parse cache hit +5ms
  vite:sfc /src/components/HelloWorld.vue template cache hit +1ms
  vite:rewrite (cached) /src/components/HelloWorld.vue?type=template +6ms
  vite:hmr ws client connected +6s
  vite:rewrite (skipped) /src/assets/logo.png +42ms
  vite:rewrite (skipped) /favicon.ico +84ms
matiasperrone commented 4 years ago

I didn't understand did it work for you?

mariusa commented 4 years ago

It doesn't work with vue3 (#213 )

I tested with vue2 and I get these: Screenshot from 2020-09-09 00-21-08 Screenshot from 2020-09-09 00-21-16

Please update the demo at https://krystalcampioni.github.io/vue-hotel-datepicker/ , you should get the same.

matiasperrone commented 4 years ago

You have to import the CSS o SCSS.

mariusa commented 4 years ago

Would you please close this issue? v4 works ok, guess we're waiting for https://github.com/krystalcampioni/vue-hotel-datepicker/issues/226#issuecomment-694319942 to publish the final 4.0.0 ?

matiasperrone commented 4 years ago

@mariusa if you have the time, I'd like to check all this "feature requests" https://github.com/krystalcampioni/vue-hotel-datepicker/milestone/2

So we can release the v4 and concentrate on v4.1 with Vue 3 support.

mariusa commented 4 years ago

Will do. Would please grant me rights to assign/remove milestones to issues?

matiasperrone commented 4 years ago

I don't think that I have that "power". It is @krystalcampioni 's power!