Tradingview chart #2463

Restioson · 2024-04-23T16:15:17Z

Fixes #2123

I'm not sure that this will work well at all DPIs - I add a 3x scale, else it is too small

TODO:

bonomat · 2024-04-24T07:08:10Z

Nice. Doesn't look too sexy though 😬 I tried it on an iOS simulator and it looks even worse :/

bonomat · 2024-04-24T08:06:08Z

Beautiful ads :D

holzeis · 2024-04-24T17:28:22Z

@Restioson can you configure the widget to open in the browser if the TradingView logo is tapped?

holzeis

If we could get rid of the in widget browser "feature" and instead open the browser on the phone it would be good to go.

Note, I haven't run it on my machine. We should check if it's properly rendered for iOS and Android.

Additionally it would be cool to recognise if the user is tilting their phone. If so we could show more details on the trading view. But that doesn't have to come with this PR.

holzeis · 2024-04-24T17:30:14Z

justfile

@@ -204,7 +204,7 @@ wipe: wipe-docker wipe-coordinator wipe-maker wipe-app wipe-webapp
 wipe-docker:
    #!/usr/bin/env bash
    set -euxo pipefail
-    docker-compose down -v
+    docker compose down -v


I fear this will break it for me since I did not install compose as plugin but as separate binary.

Hmm, on the other hand, I do not have compose and only have the plugin. According to the site the separate binary is not recommended:

I can drop this commit if we want but it'd be good to find a solution that works for both of us

Happy to change my setup if it's fine for everybody else @bonomat @luckysori

no hard feelings, on my machine both seems to work :)

I'm okay with this change.

holzeis · 2024-04-24T17:31:02Z

mobile/lib/features/trade/tradingview_candlestick.dart

+      <script type="text/javascript" src="https://s3.tradingview.com/external-embedding/embed-widget-advanced-chart.js" async>
+      {
+      "autosize": true,
+      "symbol": "BITSTAMP:BTCUSD",


🔧 can we change that to BitMEX?

mobile/lib/features/trade/tradingview_candlestick.dart

holzeis · 2024-04-24T17:33:10Z

mobile/lib/features/trade/tradingview_candlestick.dart

+    <!-- TradingView Widget BEGIN -->
+    <div class="tradingview-widget-container" style="height:100%;width:100%">
+      <div class="tradingview-widget-container__widget" style="height:calc(100% - 32px);width:100%"></div>
+      <div class="tradingview-widget-copyright"><a href="https://www.tradingview.com/" rel="noopener nofollow" target="_blank"><span class="blue-text">Track all markets on TradingView</span></a></div>


🔧 I guess we can remove that link.

We could, but I'm not sure if we would be violating some usage policy by doing that

holzeis · 2024-04-24T17:34:07Z

mobile/lib/features/trade/tradingview_candlestick.dart

+      "allow_symbol_change": false,
+      "save_image": false,
+      "calendar": false,
+      "support_host": "https://www.tradingview.com"


❓ what is the support host?

This was generated by the widget creator on their site

Restioson · 2024-04-29T12:06:16Z

Nice. Doesn't look too sexy though 😬 I tried it on an iOS simulator and it looks even worse :/

I believe it's a scaling issue. The photo that I sent looks poor because it's a JPEG. It's perfectly crisp on my phone screen but the artifacts look really bad when cropped & zoomed like that. As we've discussed with iOS it seems like the scaling algorithm isn't great for the simulator and possibly real phone too. I'm not exactly sure how we should be scaling it. Maybe there's some viewport meta tags that can be provided to specify DPI? I will have to try these out.

@Restioson can you configure the widget to open in the browser if the TradingView logo is tapped?

I can give it a shot for sure!

If we could get rid of the in widget browser "feature" and instead open the browser on the phone it would be good to go.

How do you mean - get rid of it entirely and just show a link? If we will do this, we could also use the mini ticker view and have a tap for details feature. That way there is still something to show, especially just the price and recent movements, but it won't be scrollable.

Beautiful ads :D

Do we still want to go with TradingView if we will have ads like this? 😕

holzeis · 2024-04-29T12:58:05Z

If we could get rid of the in widget browser "feature" and instead open the browser on the phone it would be good to go.

How do you mean - get rid of it entirely and just show a link? If we will do this, we could also use the mini ticker view and have a tap for details feature. That way there is still something to show, especially just the price and recent movements, but it won't be scrollable.

I meant If we can get rid of the widget opening another website internally, when clicking the TradingView logo. I think this will also fix the ad issue.

Restioson · 2024-04-29T13:26:40Z

I meant If we can get rid of the widget opening another website internally, when clicking the TradingView logo. I think this will also fix the ad issue.

Do you know anything about how the ads are showing up? I haven't had it happen yet and was assuming that it just replaced the old view (i.e without having to navigate to a new site or anything)

Restioson · 2024-04-29T13:44:00Z

I've added the functionality to only open links in a new tab in the latest commit

holzeis · 2024-04-29T18:46:14Z

Do you know anything about how the ads are showing up? I haven't had it happen yet and was assuming that it just replaced the old view (i.e without having to navigate to a new site or anything)

My theory is that the widget simply shows the tradingview.com website and that the ad is simply coming from there.

bonomat · 2024-04-30T00:05:41Z

Do you know anything about how the ads are showing up? I haven't had it happen yet and was assuming that it just replaced the old view (i.e without having to navigate to a new site or anything)

My theory is that the widget simply shows the tradingview.com website and that the ad is simply coming from there.

yea, I think the ad only came because I clicked on the trading view tab which opened trading view.

Restioson · 2024-05-01T08:27:46Z

I'm not sure we can remove the grey border, as it's inside the iframe. Same goes for the reset control. We should be able to change it to a line graph and also could edit the timezone, though. Are these features desired?

luckysori · 2024-05-01T21:40:32Z

I'm not sure we can remove the grey border, as it's inside the iframe. Same goes for the reset control. We should be able to change it to a line graph and also could edit the timezone, though. Are these features desired?

With regards to the grey border, it's not a big deal.

bonomat · 2024-05-02T05:06:52Z

I've tested it on an iPhone simulator... unfortunately it doesn't work.
I get the error Failed to open link to TradingView.
When I remove this part it works again.

..setNavigationDelegate(NavigationDelegate(
          onNavigationRequest: (req) async {
           
            final uri = Uri.parse(req.url);
            if (await canLaunchUrl(uri)) {
              await launchUrl(uri, mode: LaunchMode.externalApplication);
            } else {
              showSnackBar(ScaffoldMessenger.of(rootNavigatorKey.currentContext!),
                  "Failed to open link to TradingView");
            }

            return NavigationDecision.prevent;
          },
        ))

it looks like the requested URL is "about:blank".

Restioson · 2024-05-02T20:28:13Z

I've tested it on an iPhone simulator... unfortunately it doesn't work. I get the error Failed to open link to TradingView. When I remove this part it works again.
-- snip --
it looks like the requested URL is "about:blank".

When you say 'works', do you mean that it launches into a new browser app, or that it redirects the embedded page in the app?

Also, can you see the page fine? Is the external launching the only issue?

bonomat · 2024-05-02T23:19:27Z

I've tested it on an iPhone simulator... unfortunately it doesn't work. I get the error Failed to open link to TradingView. When I remove this part it works again.
-- snip --
it looks like the requested URL is "about:blank".

When you say 'works', do you mean that it launches into a new browser app, or that it redirects the embedded page in the app?

Also, can you see the page fine? Is the external launching the only issue?

Sorry, what I meant is that I don't see a chart at all. Only if I remove said code part I see the chart.

Restioson · 2024-05-07T08:05:40Z

Hmm, I see. I'll maybe add an exception for about scheme urls and we can see if that works? Are you happy to test this?

bonomat · 2024-05-09T00:06:14Z

Unfortunately it doesn't work on my simulator. When navigating to the trade screen it automatically opens the external browser and opens the tradingview. It only works if I remove the NavigationDelegate completely (I've tried NavigationDecision.revent and NavigateDecision.navigate. If it is on prevent the chart is not shown at all and on navigate it opens the external browser.

bonomat · 2024-05-09T00:56:52Z

Made it work for you in 60f89ff (#2463) using the same plugin we use in DLC-Connect :) Tested on Android Simulator and iOS Simulator.

Android	iOS

bonomat

Tested locally on Android and iOS simulator. I failed deploying it on the phone due to some local setting issue though, but I think we should give it a try.

This allows it to work for me. Otherwise, I get a command not found error

bonomat · 2024-05-09T05:06:24Z

@Restioson : I've rebased for you. Let's get this in.

this is also needed so that our tests run, which run on native

Restioson self-assigned this Apr 23, 2024

holzeis reviewed Apr 24, 2024

View reviewed changes

bonomat self-assigned this May 8, 2024

bonomat approved these changes May 9, 2024

View reviewed changes

Restioson added 4 commits May 9, 2024 15:03

feat: use TradingView for candlesticks

abbfdaa

fix(justfile): fix run-local-android

9bcdf43

fix(justfile): docker-compose -> docker compose

61d1511

This allows it to work for me. Otherwise, I get a command not found error

refactor: extract app background colour into const

c59ea28

bonomat force-pushed the tradingview-chart branch from 60f89ff to 28833e7 Compare May 9, 2024 05:04

fix: make trading view work using 3rd party dependency

2f8ca83

bonomat force-pushed the tradingview-chart branch from 28833e7 to 2f8ca83 Compare May 9, 2024 05:06

bonomat enabled auto-merge May 9, 2024 05:06

fix: only render on iOs and Android

8d6e9d9

this is also needed so that our tests run, which run on native

bonomat added this pull request to the merge queue May 9, 2024

Merged via the queue into main with commit d0df781 May 9, 2024
22 checks passed

bonomat deleted the tradingview-chart branch May 9, 2024 06:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tradingview chart #2463

Tradingview chart #2463

Restioson commented Apr 23, 2024 •

edited

Loading

bonomat commented Apr 24, 2024

bonomat commented Apr 24, 2024

holzeis commented Apr 24, 2024 •

edited

Loading

holzeis left a comment •

edited

Loading

holzeis Apr 24, 2024

Restioson May 1, 2024 •

edited

Loading

Restioson May 1, 2024

holzeis May 1, 2024

bonomat May 1, 2024

luckysori May 2, 2024

holzeis Apr 24, 2024

holzeis Apr 24, 2024

Restioson Apr 29, 2024

holzeis Apr 24, 2024

Restioson Apr 29, 2024

Restioson commented Apr 29, 2024

holzeis commented Apr 29, 2024 •

edited

Loading

Restioson commented Apr 29, 2024

Restioson commented Apr 29, 2024

holzeis commented Apr 29, 2024

bonomat commented Apr 30, 2024

Restioson commented May 1, 2024

luckysori commented May 1, 2024

bonomat commented May 2, 2024

Restioson commented May 2, 2024 •

edited

Loading

bonomat commented May 2, 2024 •

edited

Loading

Restioson commented May 7, 2024

bonomat commented May 9, 2024 •

edited

Loading

bonomat commented May 9, 2024 •

edited

Loading

bonomat left a comment

bonomat commented May 9, 2024

Tradingview chart #2463

Tradingview chart #2463

Conversation

Restioson commented Apr 23, 2024 • edited Loading

bonomat commented Apr 24, 2024

bonomat commented Apr 24, 2024

holzeis commented Apr 24, 2024 • edited Loading

holzeis left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Restioson May 1, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Restioson commented Apr 29, 2024

holzeis commented Apr 29, 2024 • edited Loading

Restioson commented Apr 29, 2024

Restioson commented Apr 29, 2024

holzeis commented Apr 29, 2024

bonomat commented Apr 30, 2024

Restioson commented May 1, 2024

luckysori commented May 1, 2024

bonomat commented May 2, 2024

Restioson commented May 2, 2024 • edited Loading

bonomat commented May 2, 2024 • edited Loading

Restioson commented May 7, 2024

bonomat commented May 9, 2024 • edited Loading

bonomat commented May 9, 2024 • edited Loading

bonomat left a comment

Choose a reason for hiding this comment

bonomat commented May 9, 2024

Restioson commented Apr 23, 2024 •

edited

Loading

holzeis commented Apr 24, 2024 •

edited

Loading

holzeis left a comment •

edited

Loading

Restioson May 1, 2024 •

edited

Loading

holzeis commented Apr 29, 2024 •

edited

Loading

Restioson commented May 2, 2024 •

edited

Loading

bonomat commented May 2, 2024 •

edited

Loading

bonomat commented May 9, 2024 •

edited

Loading

bonomat commented May 9, 2024 •

edited

Loading