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

[EuiDatePicker] With even more EUI #3901

Closed
1 of 3 tasks
thompsongl opened this issue Aug 12, 2020 · 9 comments
Closed
1 of 3 tasks

[EuiDatePicker] With even more EUI #3901

thompsongl opened this issue Aug 12, 2020 · 9 comments
Labels
dependencies Pull requests that update a dependency file stale-issue stale-issue-closed tech debt

Comments

@thompsongl
Copy link
Contributor

thompsongl commented Aug 12, 2020

#3476 and #3850 highlight different cases where features implemented in the vendored react-datepicker either cause problems (StrictMode warnings, from a third-party lib) or don't match the expectations of EUI consumers (popover service logic differences).

In both instances EUI provides components (EuiFocusTrap, EuiPopover) that would solve the underlying issue while providing a better overall feature experience. The current vendor structure disallows EUI components in ReactDatepicker, and changing dependencies in the vendor fork would cause even further divergence.

It's been discussed that EuiDatePicker could become a component composed of EuiFieldText, EuiPopover, and Calender (from react-datepicker). This would dramatically reduce EUI reliance on react-datepicker but would require a large amount of logic and functionality transfer (e.g., all open/close and moment parsing logic).

Want to open discussion on options to resolve open issues and decided on the future path of packages/react-datepicker/

//cc @chandlerprall

Action items:

  • move into src and merge dependencies
  • move more logic and functionality into EUI
  • pare down unneeded vendor components and dependencies
@chandlerprall
Copy link
Contributor

The right solution is probably bringing the vendored react-datepicker into src, moving its dependencies to eui's package.json, and adding the custom exceptions for jest, eslint, etc for its source code. We're only going to customize it more in the future.

A probably less effort, but future-compatible, effort would be to force react-datepicker's build to correctly map EUI components, allowing imports/usage. I think it would be straight forward for it to build with EUI, the really weird part is what happens when EUI builds: do we end up with 2 versions of EUI components or is there some magic wand to wave it away. The docs' webpack setup should be minimal but the production build seems iffy.

@cchaos
Copy link
Contributor

cchaos commented Aug 13, 2020

I'm all for reducing the amount of "skinning" we do of external libraries!

@thompsongl
Copy link
Contributor Author

the production build seems iffy

Agreed.

I'll do some more exploration, but it seems that all we "need" from react-datepicker is the Calendar component. The popover service can easily be replaced with EUI's, and all the input heavy-lifting is done by moment. The order of operations could vary, but

  • move into src and merge dependencies
  • move more logic and functionality into EUI
  • pare down unneeded vendor components and dependencies

@chandlerprall
Copy link
Contributor

All of the above sounds good to me. If we want to move the wrapping pieces like popovers into EuiDatePicker and remove them from react-datepicker, that is definitely the cleanest way for now 👍

@cchaos cchaos changed the title [Discuss] EuiDatePicker but with even more EUI [EuiDatePicker] With even more EUI Sep 20, 2020
@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@github-actions
Copy link

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

Copy link

❌ Per our previous message, this issue is auto-closing after having been open and inactive for a year. If you strongly feel this is still a high-priority issue, or are interested in contributing, please leave a comment or open a new issue linking to this one for context.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 19, 2024
@cee-chen
Copy link
Contributor

Re-opening for consideration

@cee-chen cee-chen reopened this Apr 22, 2024
@cee-chen cee-chen added dependencies Pull requests that update a dependency file tech debt labels Apr 22, 2024
@cee-chen
Copy link
Contributor

cee-chen commented May 6, 2024

Re-closing. Our long-term plans around EuiDatePicker and EuiSuperDatePicker will likely need a new meta issue. A quick note about long-terms plans:

  • Get rid of react-datepicker dependency, use EUI internals only
  • Plan for a single datepicker component instead of separate EuiDatePicker & EuiSuperDatePicker (new component entirely being worked in by Marcialis)

@cee-chen cee-chen closed this as not planned Won't fix, can't repro, duplicate, stale May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file stale-issue stale-issue-closed tech debt
Projects
None yet
Development

No branches or pull requests

4 participants