specify / specify7

Specify 7
https://www.specifysoftware.org/products/specify-7/
GNU General Public License v2.0
66 stars 36 forks source link

Edit Tree Node -> Title Lags Behind Field Value #3579

Open carlosmbe opened 1 year ago

carlosmbe commented 1 year ago

Describe the bug The Title in the Editor lags a single character behind the actual field when being changed

To Reproduce Steps to reproduce the behavior:

  1. Go to any tree node and click edit
  2. Delete a character or a few from Name and see observe the title reacts
  3. See error

Expected behavior Would be great if they were in sync

Screen Recording

https://github.com/specify/specify7/assets/53784701/17cf9f0e-207f-4a4e-a3d4-295ddd5d6563

Desktop:

melton-jason commented 1 year ago

I believe this has been an issue for quite some time (though I can't find an issue for it, if one existed). Note that if you make any changes to the Form (not just name), the Dialog title gets updated.

maxpatiiuk commented 1 year ago

Likely will be fixed by migrating off backbone

melton-jason commented 1 year ago

I believe the issue is regarding BackBone events. (Correct me if I'm wrong) We currently rely on BackBone events to monitor and perform callbacks when an event is triggered. However, even when used in a useEffect() or similar hook, either the change to the resource happens after the re-render is triggered, or some BackBone's implementation of events does not work well with React hooks.

maxpatiiuk commented 1 year ago

I think the issue is that we don't have a useEffect() at all in this place and are using resource.get('') directly inside of component render (I might be wrong, I didn't look at the code) - which is intuitive, but causes a bug.

rather than fixing this place (and many more places that have a similar issue), we should just migrate to a new resource ORM, where resource changes trigger react re-render

melton-jason commented 1 year ago

Ah, I see now (maybe)! For the ResourceView, we are getting the title from the useResourceView hook.
There is a useEffect() in the hook, but not on the Resource View component. I'm assuming the 'fix' for this would be to create a useEffect() hook in ResourceView that forces a re-render when the 'title' value from the useResourceView hook changes. Although it does seem slightly redundant, it makes sense if this is the case :sweat_smile:

https://github.com/specify/specify7/blob/80ded6ef8e95597af28c0e129471d6cdbce1a261/specifyweb/frontend/js_src/lib/components/Forms/ResourceView.tsx#L159-L173

https://github.com/specify/specify7/blob/80ded6ef8e95597af28c0e129471d6cdbce1a261/specifyweb/frontend/js_src/lib/components/Forms/BaseResourceView.tsx#L50-L69

emenslin commented 3 months ago

Can recreate in edge (7.9.6)