Nio on macOS: Mio, Part II #277

helje5 · 2021-03-16T11:37:08Z

This primarily adds displaying content using Text and composing messages using TextField, on macOS. It also has a lot of adjustments to fix visual glitches on macOS. Maybe the files should be split up per architecture instead of the sometimes relatively big #if's, not sure.

The Views we don't have on macOS yet are marked as unavailable: MarkdownView, AttributedText. The latter we probably want to have, but it is a non-trivial effort to build a good richtext compose field (the one in Shrugs is a lot of code). The former should be dropped in both iOS and macOS and get replaced with the HTML body parsing.

This is a copy of the iOS one, which we should eventually fix. According to the names, most icons should probably get replaced w/ SF Symbols anyways.

The release one is still hardcoded, like on iOS.

Follow ups to the other patch.

I know that this isn't necessary, but a) it is now consistent with my other changes :-) b) I still thing it makes the intention clearer

Mio uses more SF icons already. Maybe iOS should too.

Mark `MarkdownText` as unavailable. This approach is wrong anyways as per discussion. The text fallback in Matrix messages is _just_ text, not Markdown. For rich display we need to parse the HTML and maybe generate an attributed String from that (or a View hierarchy).

Until we have a proper attributed text field, use the plain `TextField` for message editing. It ain't great, but works. Mark `AttributedText` unavailable on macOS. And fix a linter func length warning by copying the func, linters are just the best idea evar 🤦‍♀️

The macOS version looks a little different.

A few more modifications to make stuff look nice on macOS. This also introduces a Style enum in the app file, that should probably go elsewhere.

Make sure Mio always builds.

Hope that works.

Maybe we should just merge the two.

Fails w/ ❌ error: No signing certificate "Mac Development" found: No "Mac Development" signing certificate matching team ID "Z123456789" with a private key was found. (in target 'Mio' from project 'Nio') 7 Which exceeds my GH actions knowledge.

pixlwave · 2021-03-18T10:59:07Z

Nio/Conversations/RecentRoomsView.swift

@@ -43,6 +53,47 @@ struct RecentRoomsView: View {
        rooms.filter {$0.room.summary.membership == .invite}
    }

+  #if os(macOS)


Would it be worth pulling the List out into var roomsList: some View and then having a single body that was along these lines?

#if os(macOS) roomsList .macModifiers() #else roomsList .iosModifiers() #endif

They would need to be called .archSpecificModifiers() and then have branches of that modifier. It is one way to do this if it only affects the modifiers.

RecentRoomsView was the top-level container view, right? My suggestion is to split that out into separate files as they are most likely to diverge, not sure. Actually it is often more like "is it iPad or macOS, then use the more complex view, otherwise use a small mobile view, or a watch view, even smaller"

There is also this, which I've been toying around with in a different project: sindresorhus/swiftui#7 (comment)

Sorry, yes I was kind of meaning .macModifiers() and .iosModifiers() to be shorthand for the whole list of modifiers that are currently there. I was just about to reply to

Maybe the files should be split up per architecture instead of the sometimes relatively big #if's, not sure.

to say, yes, this seems to need some consideration as it's hard to follow those big #if blocks.

Nio/Conversations/RecentRoomsView.swift

pixlwave · 2021-03-18T12:04:48Z

Did you try xcodebuild -scheme Mio build CODE_SIGN_IDENTITY="-" DEVELOPMENT_TEAM="" for the GitHub action? It succeeds locally for me.

helje5 · 2021-03-19T11:54:04Z

Did you try xcodebuild -scheme Mio build CODE_SIGN_IDENTITY="-" DEVELOPMENT_TEAM="" for the GitHub action? It succeeds locally for me.

Nope, but lets keep the CI thing out of this PR for now, it should go into a second. I'm already unhappy with myself that I even tried as part of this PR and produce a set of useless CI commits :-)

Nio/Conversations/Event Views/MessageView/BorderedMessageView.swift

NioKit/Session/AccountStore.swift

Use a proper color for the message bubbles, as available via that static function.

This is really different and doesn't belong into this PR _at all_.

pixlwave · 2021-03-19T12:17:52Z

Is it best to leave the big #if blocks as they are for now and look at them once this is merged? I'm happy either way 😄

helje5 · 2021-03-19T12:24:33Z

Is it best to leave the big #if blocks as they are for now and look at them once this is merged? I'm happy either way 😄

I think we should leave them for this iteration, i didn’t want to touch the file structure (and risk xcode project merges)

pixlwave

In which case, looks good to me! 🎉

MarcoZehe · 2021-03-20T08:34:26Z

Great work! Just tried out the current build you offered for download, and found issue #279 which is Mac specific I believe.

Prefs panel is the right place. Changing the accent color doesn't seem to do anything, though :-)

Do not wrap the form in a navigation view, put the create button into the toolbar, which surprisingly shows up proper in the sheet.

# Conflicts: # Nio/Conversations/RoomView.swift

kiliankoe · 2021-03-26T17:01:52Z

This looks awesome! I'd love to merge this or is there anything still left open?

helje5 · 2021-03-26T17:07:46Z

If everyone is cool with it, this should be merged ASAP before the other work drifts apart too far. Afterwards we should also cleanup the files (i.e. split the views w/ bigger #if sections), but only if most of the changes are in, to avoid more merge conflicts.

helje5 added 13 commits March 16, 2021 11:45

Add macOS Asset catalog

1e33e2b

This is a copy of the iOS one, which we should eventually fix. According to the names, most icons should probably get replaced w/ SF Symbols anyways.

Mio: Use bundle-id and devteam from xcconfig

c6c4de8

The release one is still hardcoded, like on iOS.

More private lets

ad012b9

Follow ups to the other patch.

Use Text(verbatim:) where appropriate

003ce59

I know that this isn't necessary, but a) it is now consistent with my other changes :-) b) I still thing it makes the intention clearer

Mio: New SFSymbol mappings

f829a65

Mio uses more SF icons already. Maybe iOS should too.

Make a macOS specific MessageComposerView

8122c53

The macOS version looks a little different.

Mio: macOS modifications for the root level views

4c1d32e

A few more modifications to make stuff look nice on macOS. This also introduces a Style enum in the app file, that should probably go elsewhere.

Add a macOS build workflow

c282f29

Make sure Mio always builds.

Use Xcode 12.4 for macOS build

4d01bd5

Hope that works.

Use a different workflow name for macOS CI

63d785e

Maybe we should just merge the two.

Drop macos.yml workflow

85dae7a

Fails w/ ❌ error: No signing certificate "Mac Development" found: No "Mac Development" signing certificate matching team ID "Z123456789" with a private key was found. (in target 'Mio' from project 'Nio') 7 Which exceeds my GH actions knowledge.

pixlwave reviewed Mar 18, 2021

View reviewed changes

Nio/Conversations/RecentRoomsView.swift Show resolved Hide resolved

pixlwave reviewed Mar 19, 2021

View reviewed changes

Nio/Conversations/Event Views/MessageView/BorderedMessageView.swift Show resolved Hide resolved

pixlwave reviewed Mar 19, 2021

View reviewed changes

NioKit/Session/AccountStore.swift Outdated Show resolved Hide resolved

helje5 added 2 commits March 19, 2021 13:02

Use .messageTextColor as requested

c53497a

Use a proper color for the message bubbles, as available via that static function.

Remove accidential commit of .synchronizable(true)

4d71654

This is really different and doesn't belong into this PR _at all_.

pixlwave approved these changes Mar 19, 2021

View reviewed changes

helje5 added 4 commits March 20, 2021 12:29

Move settings to a window on macOS

50de3a5

Prefs panel is the right place. Changing the accent color doesn't seem to do anything, though :-)

Improve the newConversation view on macOS

bad1acf

Do not wrap the form in a navigation view, put the create button into the toolbar, which surprisingly shows up proper in the sheet.

Merge branch 'stable' into feature/macos-build-2-reapply

336acf6

Merge branch 'stable' into feature/macos-build-2-reapply

f7e9ea3

# Conflicts: # Nio/Conversations/RoomView.swift

kiliankoe merged commit 788ee9e into niochat:stable Mar 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nio on macOS: Mio, Part II #277

Nio on macOS: Mio, Part II #277

helje5 commented Mar 16, 2021

pixlwave Mar 18, 2021

helje5 Mar 18, 2021

pixlwave Mar 18, 2021

pixlwave commented Mar 18, 2021

helje5 commented Mar 19, 2021

pixlwave commented Mar 19, 2021

helje5 commented Mar 19, 2021

pixlwave left a comment

MarcoZehe commented Mar 20, 2021

kiliankoe commented Mar 26, 2021

helje5 commented Mar 26, 2021

Nio on macOS: Mio, Part II #277

Nio on macOS: Mio, Part II #277

Conversation

helje5 commented Mar 16, 2021

pixlwave Mar 18, 2021

Choose a reason for hiding this comment

helje5 Mar 18, 2021

Choose a reason for hiding this comment

pixlwave Mar 18, 2021

Choose a reason for hiding this comment

pixlwave commented Mar 18, 2021

helje5 commented Mar 19, 2021

pixlwave commented Mar 19, 2021

helje5 commented Mar 19, 2021

pixlwave left a comment

Choose a reason for hiding this comment

MarcoZehe commented Mar 20, 2021

kiliankoe commented Mar 26, 2021

helje5 commented Mar 26, 2021