SP3.2 long press on rooms in space room lists #5702
Conversation
- Added context menu for spaces and rooms in Explore rooms screen
…l/5232_SP3-2_Long_press_on_rooms_in_space_room_lists
…l/5232_SP3-2_Long_press_on_rooms_in_space_room_lists # Conflicts: # Riot/Modules/Spaces/SpaceRoomList/ExploreRoom/SpaceExploreRoomViewModel.swift # Riot/Modules/Spaces/SpaceRoomList/ExploreRoomCoordinator.swift
- Small tweaks
- Added changelog
…l/5232_SP3-2_Long_press_on_rooms_in_space_room_lists # Conflicts: # Riot/Assets/en.lproj/Vector.strings
|
📱 Scan the QR code below to install the build for this PR. If you can't scan the QR code you can install the build via this link: https://i.diawi.com/HUDsck |
There was a problem hiding this comment.
Well this one was easier to review 😁
Question for @niquewoodhouse Is there any scope to use SF Symbols here instead of the images so that
a) they will scale nicely with dynamic type.
b) they will have an outlined style similar to the rest of iOS's context menus.
Riot/Modules/Spaces/SpaceRoomList/ExploreRoom/SpaceExploreRoomViewModel.swift
Outdated
Show resolved
Hide resolved
Riot/Modules/Spaces/SpaceRoomList/ExploreRoom/SpaceExploreRoomViewModel.swift
Outdated
Show resolved
Hide resolved
Riot/Modules/Spaces/SpaceRoomList/ExploreRoom/SpaceExploreRoomViewModel.swift
Outdated
Show resolved
Hide resolved
| inviteAction(for: itemData, isJoined: isJoined), | ||
| suggestAction(for: itemData, canSendSpaceStateEvents: canSendSpaceStateEvents), | ||
| isJoined ? settingsAction(for: itemData, isJoined: isJoined) :joinAction(for: itemData, isJoined: isJoined), | ||
| removeAction(for: itemData, canSendSpaceStateEvents: canSendSpaceStateEvents) |
There was a problem hiding this comment.
Seeing as the menu contents appears to be identical (to my eyes) I'm assuming that having a forRoom and forSpace methods is for future work?
There was a problem hiding this comment.
I made those 2 methods for future work indeed but I can merge them into one method if you prefer.
There was a problem hiding this comment.
No no, was only double checking in case it wasn't, it's totally fine as is 🙂
…ViewModel.swift Co-authored-by: Doug <6060466+pixlwave@users.noreply.github.com>
- Update after review
Is this something we've done for other context menus already? It does feel like they should all work the same, so however what we've already got there works is ok with me. @pixlwave do we have any options to customise the symbols or introduce our own if we can't find matches? |
Well we did this for the new context menus on the Home tab, yes.
Yes and yes. You can download the library browser here if you don't already have it: From here you can export an existing symbol, modify it and import it back to validate/organise. I think this is also the easiest way to create a new symbol by using an exported file as a template. Also worth saying that one or two of the choices in the home menu weren't ideal, we settled on those as the fastest fix for enabling Xcode 13 with the intention to review them in the future. |
gileluard
deleted the
gil/5232_SP3-2_Long_press_on_rooms_in_space_room_lists
branch
Cool, then I'm happy to use SF symbols instead. @gileluard we could pick up changing icons as part of this issue together? |
|
@niquewoodhouse the PR is already merged and the issue closed. Can we create a new issue? |
fix #5232
