feat: brag the bag #2060

bonomat · 2024-02-19T11:34:08Z

Share your current pnl from your running position as an image.

Images are dynamically fetched from my github repo here: https://github.com/bonomat/memes/tree/main/images
meaning, we can dynamically add new images any time :)

Simulator.Screen.Recording.-.iPhone.15.Plus.-.2024-02-19.at.12.30.21.mp4

resolve https://github.com/get10101/meta/issues/323

luckysori

Nice work! I just suggested a couple of tweaks.

luckysori · 2024-02-22T01:49:06Z

mobile/lib/features/brag/github_service.dart

+
+  Future<List<Meme>> fetchMemeImages() async {
+    final response =
+        await http.get(Uri.parse('https://api.github.com/repos/bonomat/memes/contents/images/'));


❓Why not create a repo under get10101? It doesn't matter, but I'm curious.

No particular reason.

I've moved it https://github.com/get10101/memes

luckysori · 2024-02-22T01:54:08Z

mobile/lib/features/brag/brag.dart

+                        mainAxisAlignment: MainAxisAlignment.center,
+                        children: [
+                          Text(
+                            "I’m trading self-custodial and without counterparty risk at 10101",


Suggested change

"I’m trading self-custodial and without counterparty risk at 10101",

"I’m trading self-custodially and without counterparty risk at 10101",

luckysori · 2024-02-22T01:57:09Z

mobile/lib/features/brag/brag.dart

+                          Row(
+                            children: [
+                              const Text(
+                                "Side",


🔧 I think this title is confusing. E.g. 2x Long should stand by itself.

Mhm, I don't like it, now it's just floating around with out any context.

luckysori · 2024-02-22T02:00:32Z

mobile/lib/features/brag/brag.dart

+                            child: Row(
+                              children: [
+                                Text(
+                                  pnlPercent.toString(),


🙃 I would omit the percentage. It's kind of boring and meaningless for anyone looking at the meme, compared to the PNL in sats.

Isn't the percentage the interesting thing? Like, I won 413 sats, vs. I made 10,123% profit?

luckysori · 2024-02-22T02:02:03Z

mobile/lib/features/brag/brag.dart

+                                  style: primaryTextStypeValue,
+                                ),
+                                Icon(
+                                  BitcoinIcons.satoshi_v2,


🔧 I have to say I still hate the sats symbol lol. I would just say sats and avoid any possible confusion.

luckysori · 2024-02-22T02:04:54Z

mobile/lib/features/brag/brag.dart

+                              children: [
+                                Text(
+                                  pnl.formatted(),
+                                  style: primaryTextStypeValue,


🔧 I like the dynamic colour on the PNL, but I would keep the other elements in a neutral colour.

luckysori · 2024-02-22T02:08:57Z

mobile/lib/features/brag/brag.dart

+                                child: Row(
+                                  children: [
+                                    const Text(
+                                      "Entry price",


🙃 The entry price is kind of interesting, but I might just omit it too in favour of keeping the image less cluttered.

I think the story the user might want to tell is something as simple as "I went long on Bitcoin at leverage 50 and I made this much".

Fair, I'll remove it

luckysori · 2024-02-22T02:12:40Z

mobile/lib/features/brag/brag.dart

+                                borderRadius: const BorderRadius.all(Radius.circular(5))),
+                            child: Padding(
+                              padding: const EdgeInsets.all(2.0),
+                              child: SvgPicture.asset('assets/10101-qr-transparent.svg',


🙃 I think the meme is a little bit too obscured (looking at the video). It would be ideal if the meme could be a bit more visible with its original colours.

I would particularly avoid covering the text with our overlay. If we remove some of the elements I suggested I think this would be possible.

I would particularly avoid covering the text with our overlay. If we remove some of the elements I suggested I think this would be possible.

This needs to be considered when creating the meme images. I've updated them which looks better now.

luckysori · 2024-02-22T02:14:33Z

mobile/lib/features/brag/brag.dart

+    "https://github.com/bonomat/memes/blob/main/images/laser_eyes_portrait.png?raw=true",
+    "https://github.com/bonomat/memes/blob/main/images/leoardo_cheers_portrait.png?raw=true",
+    "https://github.com/bonomat/memes/blob/main/images/do_something_portrait.png?raw=true",
+    "https://github.com/bonomat/memes/blob/main/images/got_some_sats_portrait.png?raw=true",
+    "https://github.com/bonomat/memes/blob/main/images/are_you_winning_son_always_have_been_portrait.png?raw=true"


❓Why do we need to specify the memes here? I thought we would dynamically fetch them from the repository. Doesn't this mean that we have to make a code change if we want to add a new meme?

Good catch, that's outdated and was just for testing.

holzeis · 2024-02-23T11:07:09Z

mobile/lib/features/brag/github_service.dart

+import 'dart:convert';
+import 'package:http/http.dart' as http;
+
+class GitHubService {


Nit: The fact that the memes are fetched from GitHub is actually irrelevant to this service.

How about MemeService?

holzeis · 2024-02-23T11:09:41Z

mobile/lib/features/brag/brag.dart

+              height: 10,
+            ),
+            Container(
+              color: Colors.transparent,


❓ is this doing anything? 😅

You don't like invisible boxes?

holzeis · 2024-02-23T11:14:43Z

mobile/lib/features/trade/trade_dialog.dart

            },
-            child: const Text("Share on Twitter")),
+            child: const Text("Share as image")),


nit: I like the subtle share icon like on the position card better. It could be an improvement over this button as well.

This dialog needs some more love... I'll leave it as is for now.

holzeis · 2024-02-23T11:16:02Z

mobile/pubspec.lock

@@ -145,6 +145,14 @@ packages:
      url: "https://pub.dev"
    source: hosted
    version: "2.1.0"
+  card_swiper:


Nit: we should probably as well replace the carousel with that.

I agree, this one is much better

Share your current pnl from your running position as an image.

bonomat requested review from holzeis and luckysori February 19, 2024 11:34

github-actions bot assigned bonomat Feb 19, 2024

bonomat force-pushed the feat/brag-the-bag branch from 848b5fc to 0c58b44 Compare February 21, 2024 14:21

luckysori approved these changes Feb 22, 2024

View reviewed changes

holzeis approved these changes Feb 23, 2024

View reviewed changes

bonomat force-pushed the feat/brag-the-bag branch from 0c58b44 to 41e5dad Compare February 23, 2024 12:43

bonomat added 2 commits February 25, 2024 20:03

feat: brag the bag

31ceeb0

Share your current pnl from your running position as an image.

feat: share from closign dialog

9da5762

bonomat force-pushed the feat/brag-the-bag branch from 41e5dad to 9da5762 Compare February 25, 2024 19:03

bonomat enabled auto-merge February 25, 2024 19:03

bonomat added this pull request to the merge queue Feb 25, 2024

Merged via the queue into get10101:main with commit 1184c02 Feb 25, 2024
20 checks passed

bonomat deleted the feat/brag-the-bag branch February 25, 2024 19:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: brag the bag #2060

feat: brag the bag #2060

bonomat commented Feb 19, 2024

luckysori left a comment

luckysori Feb 22, 2024

bonomat Feb 22, 2024

bonomat Feb 23, 2024

luckysori Feb 22, 2024

luckysori Feb 22, 2024

bonomat Feb 23, 2024

luckysori Feb 22, 2024

bonomat Feb 23, 2024

luckysori Feb 22, 2024

luckysori Feb 22, 2024

luckysori Feb 22, 2024

bonomat Feb 23, 2024

luckysori Feb 22, 2024

bonomat Feb 23, 2024

luckysori Feb 22, 2024

bonomat Feb 22, 2024

holzeis Feb 23, 2024 •

edited

Loading

holzeis Feb 23, 2024

bonomat Feb 23, 2024

holzeis Feb 23, 2024

bonomat Feb 23, 2024

holzeis Feb 23, 2024

bonomat Feb 23, 2024

	"I’m trading self-custodial and without counterparty risk at 10101",
	"I’m trading self-custodially and without counterparty risk at 10101",

feat: brag the bag #2060

feat: brag the bag #2060

Conversation

bonomat commented Feb 19, 2024

luckysori left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holzeis Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holzeis Feb 23, 2024 •

edited

Loading