Metadata
- Source
- FLUID-5607
- Type
- Bug
- Priority
- Major
- Status
- Open
- Resolution
- N/A
- Assignee
- N/A
- Reporter
- Justin Obara
- Created
2015-03-20T13:00:50.801-0400 - Updated
2015-06-22T15:10:25.802-0400 - Versions
- N/A
- Fixed Versions
- N/A
- Component
-
- Tooltip
Description
The tooltip component has several TODO's scattered throughout the codebase, as it was waiting on FLUID-3674 and FLUID-4890.
The TODO's should be addressed, now that those issues have been resolved in master. Any other modernization that can happen at this time should also be implemented.
- Currently the modelListener is setup to listen to changes on the entire model, but should listen separately to changes on "content" and "idToContent"
- remove the "initialised" member workflow and replace with a framework facility
- consider removing backwards compatibility code
Attachments
Comments
-
Simon Bates commented
2015-06-15T15:09:26.742-0400 I started looking at the following point:
- remove the "initialised" member workflow and replace with a framework facility
However, after chatting with Antranig, he suggests that this issue be put on hold until after https://github.com/fluid-project/infusion/pull/591 is merged.
The following are notes on where I got to.
"initialised" is referenced in Tooltip.js at line 130:
that.initialised = false; // TODO: proper framework facility for this coming with FLUID-4890
The pull request with the code for FLUID-4890: https://github.com/fluid-project/infusion/commit/f5403d5027923475e0c4314005f6723d2e732759
That pull request stops events from being fired on destroyed components.
I tried to see if we can simply remove the "initialised" member and (with one small change in fluid.tooltip.setup) the tests all pass with "initialised" removed. However, I do not believe that the existing tests cover all the flows that "initialised" guards.
To continue this work, I think the next step would be to understand the framework changes that have happened since the "initialised" was added to the Tooltip to see if it is actually still needed. And add unit tests as needed.
I'll attach a diff on Tooltip.js that comments out usage of "initialised" and makes the small change to fluid.tooltip.setup as mentioned above.