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

Добавлена возможность поделиться постом #410

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

SokolovaDaria
Copy link
Contributor

@SokolovaDaria SokolovaDaria commented Oct 6, 2024

resolves #404

@SokolovaDaria SokolovaDaria changed the title Сделала возможность поделиться постом. #404 resolve #404 Oct 6, 2024
@KriseevM
Copy link
Contributor

KriseevM commented Oct 6, 2024

Плиз, не меняй больше заголовки вот так, название должно словами быть всё-таки)
А resolves #404 надо в описании реквеста писать, не в названии)

@SokolovaDaria SokolovaDaria changed the title resolve #404 Добавлена возможность поделиться постом Oct 6, 2024
@SokolovaDaria
Copy link
Contributor Author

Плиз, не меняй больше заголовки вот так, название должно словами быть всё-таки) А resolves #404 надо в описании реквеста писать, не в названии)

Хорошо)

Copy link
Contributor

Choose a reason for hiding this comment

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

Подозреваю, это из другого реквеста? (из #407 ?)
Вообще, так делать не стоит
Перед работой над новой задачей надо переключаться на develop и уже от неё делать новую ветку

Copy link
Contributor

Choose a reason for hiding this comment

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

Если хочешь, можешь попробовать посмотреть на git revert, чтобы откатить изменения в этом файле) Хэши коммитов у тебя в гитхабе написаны

Copy link
Contributor

Choose a reason for hiding this comment

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

ладно, в той ветке вероятно еще будут изменения, так что настоятельно рекомендую этот один коммит откатить в этой ветке

List<AttachedFileViewModel> attachedFiles,
) async {
final String text =
'$authorName\n${DateFormat('d MMMM yyyy, HH:mm', 'ru_RU').format(postTime)}\n\n$postText';
Copy link
Contributor

Choose a reason for hiding this comment

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

Андрею такое точно бы не понравилось :D
Вынеси вызов DateFormat(<...>).format(<...>) в отдельную переменную
И еще для такого сорта строк можно использовать тройные кавычки (''' - три раза по одинарной) и тогда можно будет ставить человеческие энтеры вместо \n

Не уверен, что здесь это прям лучше будет, так что пункт про тройные кавычки - на твоё усмотрение

final String text =
'$authorName\n${DateFormat('d MMMM yyyy, HH:mm', 'ru_RU').format(postTime)}\n\n$postText';

final List<XFile> xFiles = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

final List<XFile> xFiles = [];

for (final fileViewModel in attachedFiles) {
final File? file = await fileViewModel.getFile();
Copy link
Contributor

Choose a reason for hiding this comment

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

Тут ведь файлы должны скачиваться, очень долго наверно делиться постами с большим числом файлов. И наверно будет классно, когда несколько раз поделиться нажмёшь, пока они качаются

Надо потестить будет

}
}

// ignore: use_build_context_synchronously
Copy link
Contributor

Choose a reason for hiding this comment

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

Мы обычно это обходим через if (context.mounted)

@@ -260,6 +297,42 @@ class _FeedPostState extends State<FeedPost> {
}
}

void sharePressed(
Copy link
Contributor

Choose a reason for hiding this comment

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

я бы сам метод сделал Future<void> sharePressed или даже Future<ShareResult> sharePressed
Перед Share.share и Share.shareXFiles добавил бы await (если сделаешь тип Future<ShareResult> то вообще return await Share.<...>)
И там, где его вызываешь сделал бы await sharePressed. Если будешь делать с ShareResult, то снаружи как-то обработай этот результат.
И наверно не мешало бы отловить исключения, хотя бы самого Share. Но старайся блоком try...catch охватывать как можно меньше кода. И при исключении используй LoggerService для сообщений об ошибке

Share.shareXFiles(
xFiles,
text: text,
sharePositionOrigin: box.localToGlobal(Offset.zero) & box.size,
Copy link
Contributor

Choose a reason for hiding this comment

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

@Namxobick у нас приложение под айпады работает?
image

https://pub.dev/documentation/share_plus/latest/share_plus/Share/shareXFiles.html

@KriseevM
Copy link
Contributor

KriseevM commented Oct 6, 2024

Надо копию ветки себе сделать, очень удобно смотреть реальный текст постов со всеми bb тегами можно)
Это, кстати, баг, который надо исправлять, но я пока не знаю как.
У меня где-то в коде (вроде в какой-то из вьюмоделей) вроде была регулярка для определения bb тегов, возможно стоит ей проходить и все совпадения убирать

@KriseevM
Copy link
Contributor

KriseevM commented Oct 6, 2024

и все совпадения убирать

Хотя не, убирать надо только сами теги, их содержимое оставлять, кроме картинок и [DISK], там всё убирать надо, иначе куча рандомных ссылок в сообщении останется

@KriseevM
Copy link
Contributor

KriseevM commented Oct 6, 2024

Ещё, если инфа о файле не загрузилась ещё (т.е. если там есть вот эта градиентная штука) - он не прикрепляется, Таким образом может прикрепиться 2 файла из 3, например. Советую для такой ситуации ждать, пока информация о файле скачается (вроде у вьюмодели методы есть для этого, но это не точно. Надеюсь, активное ожидание делать не придётся), но тут уже надо показывать какой-то индикатор ожидания, чтобы было понятно, что кнопка сработала, надо просто ждать

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.

Возможность поделиться постом в соцсети
3 participants