chore(Communities/InDropdown): Performance improved #14302
Conversation
…es to be joined The new property is added to allow indicating which roles from the right model should be used. It prevents from unnecessary renamings. Closes: #14298
Earier attempts to initialize proxy may fail bc left/right models may be not ready when they are set. It may lead to a situation where the model remains uninitialized and reporint 0 row count.
- removed multiple filtering separately for each category - no instantiation of all delegates up front (regular ListView approach used) - fixed category selection when filtering - selecting/deselecting only items fulfilling search criteria - significant simplification of the code Closes: #14275
micieslak
requested review from
noeliaSD,
caybro,
alexandraB99,
alexjba and
jrainville
Jenkins Builds
|
| qWarning().noquote() | ||
| << QString("Role to join %1 not found in the right model!") | ||
| .arg(roleName); | ||
| return; |
There was a problem hiding this comment.
Shouldn't it just continue here with the next role?
There was a problem hiding this comment.
Yeah, I think that both variants are ok. This one is more strict and kind of fail-fast approach. If you declared roles it's presumed that you really need them, otherwise proxy won't be initialized.
There was a problem hiding this comment.
I'd still like to see the continue here, just to be more resilitent to backend model changes (e.g. when an unused role gets dropped)
There was a problem hiding this comment.
I don't perceive it as resilience - rather easy way to overlook subtle bugs. If model is not initialized it should be easily detected by e.g. high-level squish tests bc it won't be possible to set channels and complete permission flow using channels. If the role is silently skipped it may easily introduce long-lasting small bug in the ui like e.g. incorrect names rendering.
I can change it if you insist but please consider all pros/cons here :)
There was a problem hiding this comment.
Ok, fair enough, I think it's good to stay on the conservative side 😊
|
@caybro @micieslak , something to include on this pr?? |
Yup, will add it |
Done |
What does the PR do
LeftJoinModel:InDropdown.InDropdown:used)
items fulfilling search criteria
Closes: #14298
Closes: #14275
Affected areas
LeftJoinModel,InDropdownScreenshot of functionality (including design for comparison)
Kazam_screencast_00104.webm