Fix some search issues #16943
Fix some search issues #16943
Conversation
jrainville
requested review from
micieslak,
caybro,
alexjba and
a team
as code owners
Jenkins BuildsClick to see older builds (51)
|
|
I fixed the emoji rendering in a separate commit: 092d357 |
|
"Chat" no longer has submenus (this PR vs. master):
Another problem, in both branches, the list of Status CC channels is very incomplete |
jrainville
force-pushed
the
fix/search-issues
branch
from
092d357 to
4abdfef
Compare
@caybro nice finds. I fixed in my second commit. You'll need to reset the branch |
caybro
force-pushed
the
fix/search-issues
branch
from
4abdfef to
db66f4c
Compare
|
@micieslak @osmaczko friendly reminder to review |
jrainville
force-pushed
the
fix/search-issues
branch
from
db66f4c to
1575ef7
Compare
jrainville
force-pushed
the
fix/search-issues
branch
from
1575ef7 to
167f735
Compare
|
@alexjba you've been randomly chosen to test this
|
There was a problem hiding this comment.
LGTM! I have two findings below, but looks to me like it's not related to this PR. Please let me know what you think of these:
- One community icon doesn't show in the search context menu
https://status.app/c/GzwAAGTGNrOXuOplX3Esx8vOUxjSqDQOVVEcttR_-H5JaYfvX85J_TjqcfXD_qnD37kypONGHKV-3ZiQiwI=#zQ3shnortLaN5azfhAUYM6Abmqyjp9NP3Wq3nH38DibS6Xzqz
- Chats sorting order is different in the context menu
Weird. I would expect them to either all work or all fail. It seems like it also doesn't have chats. I'll check if I can reproduce, but I'm not sure if I caused that.
Ah, the issue was only about the community order. I'll check real quick if I can get the same order for the personal chats. I think it uses the last message timestamp, so maybe it's harder. |
jrainville
force-pushed
the
fix/search-issues
branch
from
167f735 to
e53de77
Compare
|
@alexjba I fixed the ordering of the personal chat (see last commit). I also found a small bug with the color of the channels with emojis. I fixed that too. As for the missing icon on the community, I couldn't reproduce. Can you check if you can see it on master too? I haven't touched the section items, only the subitems (chats), so I don't know how I could have done that. |
Looks good on master. I can have a look tomorrow if you can't reproduce 
|
|
It's probably due to the condition here https://github.com/status-im/status-desktop/pull/16943/files#diff-503ac13f5e21b6a8dc452952c801a2239dd091d538cc9e092ca0cc90bd3b31fcR74 Found a way to reproduce: |
|
But on top of this, I see a new regression on the group chats icons 
|
jrainville
force-pushed
the
fix/search-issues
branch
from
e53de77 to
60ceb07
Compare
|
Nice finds @alexjba I fixed the group chat issue by adding a new property called As for the community issue, I just make it so that you can only search communities you actually joined. This in line with the original ticket too. |
- we have a dedicated asset category for them; makes no sense to try to parse the path to the emoji file and treat it as an (SVG) image - also fix the signal calls; over time more params were added and not all the calls were adjusted - fix selecting the "Chats" category, `model.colorHash.toJson()` is not something we can do directly in QML :) - fix group chat images
jrainville
force-pushed
the
fix/search-issues
branch
from
60ceb07 to
7ea8d43
Compare
What does the PR do
Fixes #10184
Missing part: the emoji size wasn't fixed as I need help
Affected areas
Location menu in the search popup
Architecture compliance
My PR is consistent with this document: Status Desktop Architecture Guide
Screenshot of functionality (including design for comparison)
Fixed order:
Spectated community not appearing:
Encrypted channels don't show:
Impact on end user
Fixes the location menu
How to test
Risk
Tick one:
Worst case the order of channel could still be messed up