fetrarij / ngx-daterangepicker-material

Pure Angular 2+ date range picker with material design theme, a demo here:
https://fetrarij.github.io/ngx-daterangepicker-material/
MIT License
248 stars 251 forks source link

Use CDK Overlay to display the date-range-picker #252

Closed Timebutt closed 4 years ago

Timebutt commented 4 years ago

To be in line with other Angular Material components, I feel the date-range-picker should be rendered using the CDK Overlay instead of the custom position: absolute <div> we have now.

I am trying to incorporate this component in my application, but the look and feel stand out too much from other Material components for me to use it. The CDK has some wonderful features that we would get to leverage for free (positioning, Material-like UX, ...), so I feel it'd be a major improvement.

I submitted a PR upgrading this repo to Angular 9 (https://github.com/fetrarij/ngx-daterangepicker-material/pull/251) to allow us to use the latest features the CDK offers, and would be willing to take a stab at changing the way this component renders.

Opinions?

Timebutt commented 4 years ago

For reference, I have started work on this and made some major improvements to this project. The main problem I have with this repository, is that it doesn't leverage Material in the correct way (select component, buttons, ...). The materialize-css package throws off what the input fields should look like, so instead: I reimplemented the component using the correct Angular Material components. This became apparent when trying to wrap the daterangepicker in a mat-form-field (a feature every Material input should support).

I also had issues with z-index when using the component in my own project, which is why I opened this issue to use the Material CDK Overlay, that has meanwhile been implemented too.

I have deployed a first version of the updated component and documentation here, have a look: https://timebutt.github.io/ngx-daterangepicker-material/simple.

The branch can be found here: https://github.com/Timebutt/ngx-daterangepicker-material/tree/use-material-select

My proposal is much more in line with how a Material component should look and feel like. There are still some issues I have to address before this can be merged but I'm close to wrapping up.

fetrarij commented 4 years ago

@Timebutt wow this is great. But does not it break the standard usage of the component? I mean if user doesn't choose to use angular material, because the component is done to be independant.

Timebutt commented 4 years ago

Hey @fetrarij that's a great question, but I wonder who wants to use a Material component in an Angular application without it being the real deal? If you're building an application with Material UI, I don't see why you wouldn't use the officially supported @angular/material library for it? Apart from that: the current implementation doesn't support theming, which will always require overrides and decreases the usability IMO.

I see these scenarios ways moving forward:

I think it would be very interesting to hear from the community on what would be the best way forward for people using this library. My personal opinion is that moving towards full @angular/material support will greatly increase the adoption of this library, sparking further interest from the community to collaborate on this project. In its current state, I cannot leverage the component in my application (and I don't think anyone that uses @angular/material in their project can), which is why I put in the effort to get it up to par.

Eager to hear your feedback, and mostly feedback from other contributors or the community in general!

aylmercarson commented 4 years ago

i concur with tb. i think that any component which claims to target material should not try to be material-agnostic. i am running into some UI issues myself in a purely @angular/material dev environment which it would be great to see removed. but this is not to decry the fantastic effort put in by fet. this has the potential of being an amazingly useful component and i really appreciate it.

Timebutt commented 4 years ago

@aylmercarson I have meanwhile almost finished work on the implementation, you can try it out here. There are still a few minor things I want to iron out to make it as close to perfect as I can get it, but I already use it in production in my own application for instance.

I haven't published the upgraded version of this component yet, but you can install it to try it out by pulling my branch, building it (npm i and then npm run build) and then point ngx-daterangepicker-material in your application's package.json to the dist folder of where you cloned my fork (example in my case: "ngx-daterangepicker-material": "file:../ngx-daterangepicker-material/dist",). Let me know!

aylmercarson commented 4 years ago

@Timebutt hi, thanks for that. i have installed, built and referenced. this certainly fixes layout issues i had but i still have an issue with opacity. the popup appears transparent. this may well be due to some of my other css though so far, i've been unable to track it down. but again, thanks for this :)

Timebutt commented 4 years ago

@aylmercarson if that's the case, it's probably because you didn't set the Material theme up correctly. As you can see from the examples in the documentation I wrote, the background color is taken from the material theme. Have you correctly installed a theme (prebuilt or custom)? The Angular Material Guide on themes provides some very clear instructions on how to do that.

aylmercarson commented 4 years ago

@Timebutt i'll check it out. i currently have

@import "~@angular/material/prebuilt-themes/indigo-pink.css";

in my styles.scss file

fetrarij commented 4 years ago

@Timebutt after thinking about it, i think it's a good idea, i will accept your PR if you want. I will create a new tag for that.

Timebutt commented 4 years ago

That's great news! I even fixed some issues I found in the current master while at it, plus massively improved the typing of both the directive and component. Only thing I see that still needs improvement is the fact that the code examples don't render very well on both smaller screens and in dark mode, so I'll see if I can finetune that.

Oh and I'll need to rewrite/adjust some of the documentation to reflect current state of affairs. I'll create a PR already so that you can have a look at what I've changed.

Dhiraj3089 commented 4 years ago

@Timebutt will CDK overlay fix work with angular 8 ?? currently I am using ngx-daterangepicker-material 2.1.9

Timebutt commented 4 years ago

@Dhiraj3089 absolutely!

Dhiraj3089 commented 4 years ago

@Timebutt Great 👍 . But I am facing an issue when I put datepicker inside material dialog. If dialog is too small then datepicker popup is not working as expected. It opens inside the dialog instead of popping outside the box. I have reproduced the scenario on stackblitz

Thanks in advance

Timebutt commented 4 years ago

Well that's exactly why I propose to reimplement the date range picker to leverage the CDK as I was having similar issues. I left some instructions above that allow you to already try my changes, I have yet to publish a beta package as I need some more work to fine tune everything. Could you check and see if that solves your issues?

fetrarij commented 4 years ago

@Timebutt thanks for your effort..I am now testing

Timebutt commented 4 years ago

Closing this issue as this was resolved by merging https://github.com/fetrarij/ngx-daterangepicker-material/pull/257.

kimlambiguitoppolis commented 1 year ago

image

not sure if the change at #257 was merged to v6 but looking at the generated html, it doesn't seem like it. that's probably why issues #517 and #508 still happens, similar to what is happening in the stackblitz link on this comment. #252

Timebutt commented 1 year ago

Hey @kimlambiguitoppolis, the Material upgrade I did was reverted in version 4.0.0 because some people didn't like adding @angular/material as an extra dependency to a library that aims to be a Material component ;)

If you want you can use version 3.0.1 which still uses the official CDK, or you can use the package I published recently from my fork over at https://github.com/Timebutt/ngx-daterangepicker-material which supports Angular 15 (I use this extensively in my own projects). Package can be found here. Let me know!

kimlambiguitoppolis commented 1 year ago

Hey @kimlambiguitoppolis, the Material upgrade I did was reverted in version 4.0.0 because some people didn't like adding @angular/material as an extra dependency to a library that aims to be a Material component ;)

If you want you can use version 3.0.1 which still uses the official CDK, or you can use the package I published recently from my fork over at https://github.com/Timebutt/ngx-daterangepicker-material which supports Angular 15 (I use this extensively in my own projects). Package can be found here. Let me know!

Thank you sir. That's sad but I'll try your version out.

paulflo150 commented 10 months ago

@Timebutt do you have any plans to update your fork to luxon? Thanks!

Timebutt commented 10 months ago

No plans right now no @paulflo150 ;)