Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Part 1] Build linux version #1730

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

[Part 1] Build linux version #1730

wants to merge 18 commits into from

Conversation

Te-Z
Copy link
Contributor

@Te-Z Te-Z commented Apr 25, 2024

Ticket: #1699

To do:

Fixed:

  • SSO login
  • App bar click
  • Download files:
    • files are downloaded in a Twake Chat folder inside system's download folder
    • pictures and videos are downloaded in download folder
  • Media viewer display
  • copy text
  • paste text
  • change avatar
  • send files
  • shortcuts (select all)
Capture.video.du.06-05-2024.15.52.00.webm

@Te-Z
Copy link
Contributor Author

Te-Z commented Apr 25, 2024

Stuck with SSO login for now. I'm trying to update flutter_web_auth_2 (some alpha versions are available) to see if it resolve something but I did not found any solution in its documentation.

Capture.video.du.25-04-2024.10.19.14.webm

@hoangdat
Copy link
Member

  • IMO, after app execute OIDC, matrix callback to linux app, but no one can handle it -> block like your video. And I did not found any dependencies support oidc for linux.
  • we should take a look on: https://github.com/krille-chan/fluffychat, how they handle the issue?

@hoangdat
Copy link
Member

pubspec.yaml Outdated Show resolved Hide resolved
@Te-Z Te-Z force-pushed the TW-1699/build_linux_version branch 2 times, most recently from 8201383 to 4d93694 Compare April 26, 2024 13:32
Copy link

This PR has been deployed to https://linagora.github.io/twake-on-matrix/1730

@sherlockvn
Copy link
Collaborator

shortcuts

@Te-Z Te-Z force-pushed the TW-1699/build_linux_version branch from 5e783c0 to b7cec7a Compare May 6, 2024 13:38
@Te-Z Te-Z marked this pull request as ready for review May 6, 2024 13:43
@Te-Z Te-Z requested review from sherlockvn and dab246 as code owners May 6, 2024 13:43
@Te-Z Te-Z changed the title [DRAFT] Build linux version [Part 1] Build linux version May 6, 2024
)
.copyWith(letterSpacing: -0.15),
Widget _buildInputBar(BuildContext context) {
return Shortcuts(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work on Web platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capture.video.du.14-05-2024.11.40.35.webm

@@ -54,39 +52,33 @@ class ChatInputRowWeb extends StatelessWidget {
Expanded(
child: inputBar,
),
KeyBoardShortcuts(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Shortcuts are now in _buildInputBar which is a child widget of ChatInputRowWeb. This allow to use shortcuts whenever the input bar is used. Also this widget did not work. My thought is that it used the wrong focus scope when put inside chat_input_row_web.dart

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the related lines:

ChatInputRowMobile _buildMobileInputRow(BuildContext context) {
return ChatInputRowMobile(
inputBar: Column(
children: [
ReplyDisplay(controller),
_buildInputBar(context),
],
),
emojiPickerNotifier: controller.showEmojiPickerNotifier,
onEmojiAction: controller.onEmojiAction,
onKeyboardAction: controller.onKeyboardAction,
);
}
ChatInputRowWeb _buildWebInputRow(BuildContext context) {
return ChatInputRowWeb(
inputBar: Column(
children: [
ReplyDisplay(controller),
_buildInputBar(context),
],
),
emojiPickerNotifier: controller.showEmojiPickerNotifier,
onTapMoreBtn: () => controller.onSendFileClick(context),
onEmojiAction: controller.onEmojiAction,
onKeyboardAction: controller.onKeyboardAction,
);
}
Widget _buildInputBar(BuildContext context) {
return Shortcuts(
shortcuts: <LogicalKeySet, Intent>{
LogicalKeySet(
LogicalKeyboardKey.controlLeft,
LogicalKeyboardKey.keyA,
): const SelectAllIntent(),
LogicalKeySet(
LogicalKeyboardKey.altLeft,
LogicalKeyboardKey.keyE,
): const OnEmojiActionIntent(),
},
child: Actions(
actions: <Type, Action<Intent>>{
SelectAllIntent: CallbackAction<SelectAllIntent>(
onInvoke: (_) => controller.selectAll(),
),
OnEmojiActionIntent: CallbackAction<OnEmojiActionIntent>(
onInvoke: (_) => controller.onEmojiAction(),
),
},
child: InputBar(
typeAheadKey: controller.chatComposerTypeAheadKey,
rawKeyboardFocusNode: controller.rawKeyboardListenerFocusNode,
room: controller.room!,
minLines: 1,
maxLines: 8,
autofocus: !PlatformInfos.isMobile,
keyboardType: TextInputType.multiline,
textInputAction: null,
onSubmitted: (_) => controller.onInputBarSubmitted(),
suggestionsController: controller.suggestionsController,
typeAheadFocusNode: controller.inputFocus,
controller: controller.sendController,
focusSuggestionController: controller.focusSuggestionController,
suggestionScrollController: controller.suggestionScrollController,
showEmojiPickerNotifier: controller.showEmojiPickerNotifier,
decoration: InputDecoration(
hintText: L10n.of(context)!.chatMessage,
hintMaxLines: 1,
hintStyle: Theme.of(context)
.textTheme
.bodyLarge
?.merge(
Theme.of(context).inputDecorationTheme.hintStyle,
)
.copyWith(letterSpacing: -0.15),
),
onChanged: controller.onInputBarChanged,
),
),
);
}
}

Comment on lines 74 to 77
final String initialDirectory =
(await getApplicationDocumentsDirectory()).path;
final List<XFile> xFiles =
await openFiles(initialDirectory: initialDirectory);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO: I saw you still using it in another place (lib/pages/settings_dashboard/settings_profile/settings_profile.dart). Can you separate this func so it can be reused?

Copy link
Member

@nqhhdev nqhhdev May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO:

  • If (await getApplicationDocumentsDirectory()).path or await openFiles(initialDirectory: initialDirectory)
    has error, how to handle error?
  • Should try catch for this case, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO: I saw you still using it in another place (lib/pages/settings_dashboard/settings_profile/settings_profile.dart). Can you separate this func so it can be reused?

Actually on lib/pages/settings_dashboard/settings_profile/settings_profile.dart the method used is openFile without the s at the end of the word file. This method force user to open only one file.

But openFiles used here allow user to select multiple files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO:

  • If (await getApplicationDocumentsDirectory()).path or await openFiles(initialDirectory: initialDirectory)
    has error, how to handle error?
  • Should try catch for this case, WDYT?

done

Comment on lines +81 to +86
final fileStoreDirectory = await StorageDirectoryManager.instance
.getFileStoreDirectory(isTemporary: isTemporary);
if (isTemporary) {
return '$fileStoreDirectory/$eventId/decrypted-$fileName';
}
return '$fileStoreDirectory/${AppConfig.applicationName}/decrypted-$fileName';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just make another method

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still confused

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/pages/connect/connect_page_mixin.dart Outdated Show resolved Hide resolved
lib/presentation/mixins/connect_page_mixin.dart Outdated Show resolved Hide resolved
lib/domain/model/extensions/xfile_extension.dart Outdated Show resolved Hide resolved
lib/pages/bootstrap/bootstrap_dialog.dart Outdated Show resolved Hide resolved
lib/pages/chat/chat.dart Outdated Show resolved Hide resolved
pubspec.yaml Show resolved Hide resolved
@hoangdat
Copy link
Member

Hi @Te-Z ,
I still have some remarks:

  • test responsive: mobile-web
  • chat event list: why need to separate mobile/web, responsive thi sao // build in all platform, demo to avoid side effect
  • chat input: build in all platform, demo to avoid side effect
  • message_content: native: mobile/desktop should base on stream, web still base on bytes
  • chat details media page
  • chat list header
  • chat search view: because related to download content: view can reuse from web (but take care responsive) but logic should base on stream
  • file picker in all platforms
  • redirect in OIDC, port?, what happen if this port is used by other service?
  • play video is laggy
  • cache location in desktop

@Te-Z
Copy link
Contributor Author

Te-Z commented May 15, 2024

Hi @Te-Z , I still have some remarks:

  • test responsive: mobile-web
  • chat event list: why need to separate mobile/web, responsive thi sao // build in all platform, demo to avoid side effect
  • chat input: build in all platform, demo to avoid side effect
  • message_content: native: mobile/desktop should base on stream, web still base on bytes
  • chat details media page
  • chat list header
  • chat search view: because related to download content: view can reuse from web (but take care responsive) but logic should base on stream
  • file picker in all platforms
  • redirect in OIDC, port?, what happen if this port is used by other service?
  • play video is laggy
  • cache location in desktop

Hi @hoangdat

  • about chat event list: I did not, it was already separated but it was done to use the SelectionArea widget on web and desktop. Here's a demo for this point and the responsive test:
    • desktop (side by side with current beta version)
Capture.video.du.15-05-2024.15.16.44.webm
  • web
Capture.video.du.15-05-2024.15.30.44.webm
  • mobile
Capture.video.du.15-05-2024.15.37.04.webm
  • chat input row:
    • desktop
Capture.video.du.15-05-2024.15.54.24.webm
Capture.video.du.15-05-2024.15.58.50.webm
  • play video is laggy: can you please be more specific ? I could not reproduce it
Capture.video.du.15-05-2024.15.51.51.webm
  • cache location in desktop: on linux: temporary files are stored at /tmp but cache is at /home/<user>/.cache

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants