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

Run search filter on item list page reinit, fixes #861 #954

Merged
merged 1 commit into from
Mar 14, 2021

Conversation

hubsif
Copy link
Contributor

@hubsif hubsif commented Mar 12, 2021

Analyzing this I found out that the searchbar search gets overriden by virtual list's replaceAllItems and has to be rerun. So, the only required change here actually is:

https://github.com/hubsif/openhab-webui/blob/085bc59f5d99b875135c35c60cff04bf1bd61d9f/bundles/org.openhab.ui/web/src/pages/settings/items/items-list-vlist.vue#L151-L154

While analyzing I stumbled across initSearchbar and alike, and this:

https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui/web/src/pages/settings/items/items-list-vlist.vue#L168-L175

To me that looked like you were struggling with rendering and loading race conditions.

So, again I did more than required and tried to clean that up. I think the main issue was in the list to be conditionally rendered and thus not available for the searchbar on init:

I changed that to v-show instead and was then able to tidy up quite some.

Additionally, I discovered a "flicker" when going back from an item detail to the list: Due to the list only loading at "pageAfterIn", the transition would still show the filtered list and when finished switch to the loading components.
I wondered why you actually only load the page at "page:afterin" instead of e.g. "page:beforeIn"?
Anyways, I found it to be best situated in "page:init" and "page:reinit", I hope I didn't miss anything.

Signed-off-by: Hubert Nusser <hubsif@gmx.de>
@hubsif hubsif requested a review from a team as a code owner March 12, 2021 17:17
@relativeci
Copy link

relativeci bot commented Mar 12, 2021

Job #24: Bundle Size — 10.88MB (~-0.01%).

6061bbb vs 1ec3222

Changed metrics (2/8)
Metric Current Baseline
Initial JS 1.59MB(-0.04%) 1.59MB
Cache Invalidation 17.07% 15%
Changed assets by type (1/7)
            Current     Baseline
JS 8.55MB (~-0.01%) 8.55MB

View Job #24 report on app.relative-ci.com

@ghys
Copy link
Member

ghys commented Mar 14, 2021

I wondered why you actually only load the page at "page:afterin" instead of e.g. "page:beforeIn"?

It was more than 2 years ago, but I seem to remember it was to let the transition finish before making replacing the skeletons with the real items, thus making big changes to the DOM and potentially causing a lag especially noticeable during slide effects.

This code was indeed quite fragile, but I can't put your changes at fault, so LGTM.

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

Successfully merging this pull request may close these issues.

2 participants