use modern UIMenu #2545
use modern UIMenu #2545
Conversation
r10s
force-pushed
the
r10s/more-modern-menus
branch
2 times, most recently
from
3ea1034 to
128e657
Compare
r10s
force-pushed
the
r10s/more-modern-menus
branch
from
128e657 to
cec2555
Compare
r10s
changed the title
r10s
force-pushed
the
r10s/more-modern-menus
branch
from
cec2555 to
216d509
Compare
r10s
changed the title
r10s
force-pushed
the
r10s/more-modern-menus
branch
from
3bc7bc9 to
e320c98
Compare
r10s
changed the title
r10s
changed the title
r10s
force-pushed
the
r10s/more-modern-menus
branch
5 times, most recently
from
48945d9 to
fb8b53d
Compare
|
puh, was a little bit more than expected - but now we have all drop-down-menus modernised :) (context menus and attach menu were modernised already before) |
r10s
force-pushed
the
r10s/more-modern-menus
branch
from
0f14436 to
52b88ab
Compare
it is not an action that is useful during usage (as 'add to widget'), and if, 'forward' should also be there. instead, just point to 'show in chat' where meanwhile both are available (which was not always the case)
| guard let self else { return } | ||
| viewModel.pinChatsToggle(indexPaths: tableView.indexPathsForSelectedRows) | ||
| setLongTapEditing(false) | ||
| }) | ||
|
|
||
| if viewModel.hasAnyUnmutedChatSelected(in: tableView.indexPathsForSelectedRows) { | ||
| alert.addAction(UIAlertAction(title: String.localized("menu_mute"), style: .default) { [weak self] _ in | ||
| actions.append(UIAction(title: String.localized("menu_mute"), image: UIImage(systemName: "speaker.slash")) { [weak self] _ in | ||
| guard let self else { return } | ||
| MuteDialog.show(viewController: self) { [weak self] duration in |
There was a problem hiding this comment.
This could return a sub-menu instead maybe?
There was a problem hiding this comment.
i also thouht about that, but the mute dialog is also available at other places, so it seems better to have the same UI everywhere
There was a problem hiding this comment.
Maybe those other places could use the menu too? 🤔 But can be another PR, this already addresses a lot of them.
There was a problem hiding this comment.
hm, at least whatsapp and signal also use an actionSheet for the concrete setting. i also have rarely seen a UIMenu in response to a swipe-to-action gesture (where mute is also available)
but yes, if, then it anyway would be another pr, so this is no blocker
There was a problem hiding this comment.
Ah yeah I don't think swipe action is a great place for it.
| UIAction(title: String.localized("proxy_use_proxy"), image: UIImage(systemName: "shield")) { [weak self] _ in | ||
| self?.showProxySettings() | ||
| }, |
There was a problem hiding this comment.
Probably more of a UX-thing, but if we only have one entry a menu, does it make a lot of sense to show that menu -- and not the menu-entry itself? In other words: Why does the BarButtonItem show a menu and not the Proxy-button?
It feels like an extra step for the user to access the proxy-settings.
There was a problem hiding this comment.
but if we only have one entry a menu, does it make a lot of sense to show that menu
yes - as the symbol is not self-explaining for most users. therefore, an additional descriptive text is better.
when proxy is then enabled, the icon is shown directly - this is also what is done in the main app.
|
thanks for review, @zeitschlag and @Amzd ❤️ |
this PR replaces
someall of the old "actionSheet" menus with the more modern ones available since iOS 13/14.it is all pretty much straight-forward. greater refactorings, renamings are not done, to keep the changed lines small.
see commit messages for more details, this is what it looks like: