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

gray out options toggle on crypto #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

miraclx
Copy link
Contributor

@miraclx miraclx commented Feb 24, 2021

Inspired by #93's behaviour with the Volumes 'v' item in the toggles, this relaxes #37 and better resolves #49.

greying out, not hide the options toggle when crypto is selected
@tarkah
Copy link
Owner

tarkah commented Feb 24, 2021

Haha you've read my mind, love it

@tarkah
Copy link
Owner

tarkah commented Feb 24, 2021

We should do the same for Pre Post 'p' in non-1D timeframes, can you just layer that into this PR?

@miraclx
Copy link
Contributor Author

miraclx commented Feb 24, 2021

Absolutely, I'm on it!

@miraclx
Copy link
Contributor Author

miraclx commented Feb 24, 2021

I went into working on this thinking Pre Post 'p' didn't do anything for all non-1D timeframes. But I just realized that wasn't the case.

What's the constraint I need to check for to disable this?

  • Is it crypto specific?
  • Or is it just non-1D timeframes? (I didn't notice any changes with pre-post, 1D timeframe for any crypto I tested with)

I guess I'm just asking, does pre-post work for crypto at all?

@tarkah
Copy link
Owner

tarkah commented Feb 24, 2021

Yeah, pre / post isn't applicable for Options Crypto because they trade 24/7. So you can have it disabled completely for options, then just non-1D for everything else.

@miraclx
Copy link
Contributor Author

miraclx commented Feb 24, 2021

Options? Do you mean crypto?

And for "everything" else, I actually notice some changes on non-crypto symbols when in non-1D time frames.

To be clear, are you saying pre-post should only work for non-crypto symbols in the 1D timeframe, because if that's the case then that's not what's happening.

@tarkah
Copy link
Owner

tarkah commented Feb 25, 2021

Options? Do you mean crypto?

And for "everything" else, I actually notice some changes on non-crypto symbols when in non-1D time frames.

To be clear, are you saying pre-post should only work for non-crypto symbols in the 1D timeframe, because if that's the case then that's not what's happening.

Yes sorry, crypto. Yeah, that's what should be happening. Can you give me some examples where pre / post is changing things for non-1D time frames or crypto?

@miraclx
Copy link
Contributor Author

miraclx commented Feb 25, 2021

It's not changing things for crypto but for everything else:

pre/post disabled pre/post enabled
Screenshot_20210225_010802 Screenshot_20210225_010826
Screenshot_20210225_012656 Screenshot_20210225_012731
Screenshot_20210225_012833 Screenshot_20210225_012854

The values change and the graph changes slightly too at the rightmost edge

@tarkah
Copy link
Owner

tarkah commented Feb 25, 2021

Ahh right, because I use the Post Market price as the last datapoint when it's toggle on vs Regular Market price when it's off, which alters the last datapoint on all the timeframes. I can easily fix that, I don't really think it makes sense to show the post-market price on the other time frames. What do you think?

@miraclx
Copy link
Contributor Author

miraclx commented Feb 25, 2021

Yeah, it doesn't strike me as a useful metric outside of that. It's only really usable in the 1D timeframe.

So with this PR, to clarify, I'll be graying out pre-post entirely for crypto and only on non-1D timeframes for others.

I should probably wait for that fix so I can rebase and push.

@tarkah
Copy link
Owner

tarkah commented Feb 25, 2021

Sounds like a plan. I'll try to tackle that sometime tomorrow.

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.

Options shortcut 'o' doesn't work for cryptocurrency
2 participants