Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Skip item element cache when search is preserved #105

Merged
merged 1 commit into from
Feb 7, 2018

Conversation

daviwil
Copy link
Contributor

@daviwil daviwil commented Feb 7, 2018

Description of the Change

This change works around an issue introduced in 0.43.0 which causes the
command palette to become unusable when the user enables the "Preserve
Last Search" setting and then searches a few times. This is caused by
the new item element caching logic which preserves items from previous
searches. Somehow these cached elements do not have their parentNode
set to the SelectListView causing the crash which breaks the command
palette.

This change is a temporary workaround to unblock the release of Atom
1.24 and 1.25-beta until the core issue can be addressed.

Related to atom/atom#16622.

Alternate Designs

Another approach was to add a null check around the use of parentNode.replaceChild in atom-select-list, but this caused other UX problems to occur. The better workaround is to avoid the broken elements to begin with. A real fix that addresses the core problem is still needed, but this change unblocks the release of 1.24 and 1.25-beta.

Benefits

No more crashing command palette which can only be resolved by restarting Atom.

Possible Drawbacks

Reduced performance when "Preserve Last Search" is turned on.

Applicable Issues

atom/atom#16622

/cc @iolsen @lee-dohm @Ben3eeE @as-cii

This change works around an issue introduced in 0.43.0 which causes the
command palette to become unusable when the user enables the "Preserve
Last Search" setting and then searches a few times.  This is caused by
the new item element caching logic which preserves items from previous
searches.  Somehow these cached elements do not have their parentNode
set to the SelectListView causing the crash which breaks the command
palette.

This change is a temporary workaround to unblock the release of Atom
1.24 and 1.25-beta until the core issue can be addressed.

Related to atom/atom#16622.
@daviwil
Copy link
Contributor Author

daviwil commented Feb 7, 2018

Going to move on with merging this and publishing an update to the package so that we can get the release process started.

@daviwil daviwil merged commit 37cd9d1 into master Feb 7, 2018
@daviwil daviwil deleted the dw-fix-atom-16622 branch February 7, 2018 20:42
@t9md t9md mentioned this pull request Feb 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant