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

[date-picker] sync the input element value and forge-date-picker value when disconnected from the DOM #821

Open
jm-boop opened this issue Feb 12, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@jm-boop
Copy link

jm-boop commented Feb 12, 2025

Describe the bug:
When the forge-date-picker is disconnected/removed from the DOM, updating the component value does not update the input value, and vice versa.

To Reproduce:
Steps to reproduce the behavior:

  1. Go to https://forge.tylerdev.io/main/?path=/story/components-date-picker--demo
  2. Open Chrome DevTools
  3. Give the forge-date-picker an id to query select (ex. id='foobar')
  4. Run each line independently in the console:
// query select the elements that are connected to the DOM
let datePicker = document.getElementById('foobar')
let parentElement = datePicker.parentElement
let input = datePicker.querySelector('input')

// set the input element's value, validating the component's value is synced
input.value = '12/31/1999'
input.dispatchEvent('input')
datePicker.value // Fri Dec 31 1999 00:00:00 GMT-0500 (Eastern Standard Time)   -- GOOD, in sync

// now disconnect/remove the date-picker from the DOM
datePicker.remove()

// update the input element's value, and see the component's value is not synced
input.value = '01/01/2000' // happy new year!
input.dispatchEvent('input')
datePicker.value // Fri Dec 31 1999 00:00:00 GMT-0500 (Eastern Standard Time)   -- BAD, out of sync

// reconnect/append the date-picker to the DOM, and notice the old value 'sticks'
parentElement.append(datePicker)

Expected behavior:
I expect the component value and input value to always be in sync, regardless if the component is connected or disconnect from the DOM.

Screenshots:

Please complete the following information:

  • Forge version: [e.g. v3.6.1]
  • I have searched existing issues before creating this report? Y
  • Browser: All
  • Platform: All
  • OS: All

Additional context:
For consumers of Forge, this can be a serious issue if JavaScript is changing the value of the input on disconnected components.
For example, with folder tabs, some frameworks will remove/disconnect the components upon tab switching, followed by updating the value of the input. This ultimately leads to old values 'sticking' when the tab containing the date-picker is clicked back into.

@DRiFTy17
Copy link
Collaborator

DRiFTy17 commented Feb 12, 2025

hmm this will be a tricky one to solve for the following reasons:

  1. These events do not bubble when elements are disconnected from the DOM because their listeners are removed when the disconnectedCallback() runs, so we can't rely on that communication channel between these components as you might expect...
  2. While setting the value on the <input> does technically work, we do typically expect the value to be set through the <forge-date-picker> element so that is can handle the syncing to the <input> rather than the other way around.

We currently allow for the input value to sync to the date picker by patching the value property on the <input>, which is not ideal, but it's the only way possible without events... The reason the issue you describe is happening is because we unpatch that value property setter on the <input> when the <forge-date-picker> is disconnected to avoid memory leaks by holding onto a patched property on an element we don't "own". We would need to leave that patch alive for this to work, but we also don't patch that property until the elements are connected so this technically wouldn't work if the elements were never added to the DOM.

Hopefully that gives you a good idea of what's going on, and why it operates the way it does currently. If we could include the <input> within the shadow DOM we certainly would, but due to accessibility reasons we are unable to do that until new browser APIs are made available to use for cross-root ARIA.

I'm not quite sure the best way to handle this quite yet, and it will likely be solved on its own in the future when we can move the inputs to the shadow DOM. Are you able to force the communication to go through the <forge-date-picker> instead?

@jm-boop
Copy link
Author

jm-boop commented Feb 13, 2025

@DRiFTy17, yes, I have a workaround to force the value of the <forge-date-picker> when the input value is set in this scenario. In the workaround below (from this PR), the super of setValue is setting the input value:

setValue(value, fromVM): void { //tslint:disable-line:require-private-modifier
      $super.setValue.call(this, value, fromVM);
      // when the <forge-date-picker> is disconnected/removed from the DOM (if leaving a folder tab that has a date picker, for example),
      // the input value gets set correctly, but the value of the component does not get synced. This is a workaround to set the
      // component value until https://github.com/tyler-technologies-oss/forge/issues/821 is resolved.
      if (!this.getElement().isConnected) {
        this.getElement().value = new Date(value);
      }
},

Why was this not an issue with TCW? Other than that, everything you said made sense, Kieran, thanks for the information!

@DRiFTy17
Copy link
Collaborator

@jm-boop The reason this worked in TCW was purely by accident actually. That version had a memory leak where it would never clean up the patch that it applied to the <input>'s value property setter. So after the date picker was removed from the DOM, the patched value setter was still calling the date picker's listener/callback from that patch, and we fixed this in a later version a while back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants