Improve charts partially resolves #80 #91
Conversation
| export enum Status { | ||
| Confirmed = 'confirmed', | ||
| Confirmed = 'active', |
There was a problem hiding this comment.
@danbilokha Do you say I'm showing wrong data to user
OR I just need to rename Enum for consistancy?
There was a problem hiding this comment.
rename for consistency, I am not sure how you use that for showing the data
| ikCbData?: string | ||
| ): Promise<TelegramBot.Message> => { | ||
| try { | ||
| const [arg1, arg2] = splitArgument(ikCbData); |
There was a problem hiding this comment.
what if it's in message.text?
There was a problem hiding this comment.
Let's discuss offline, cause I've prolly misunderstund the architecture
| @@ -1,7 +1,7 @@ | |||
| export const addDays = (date: string | Date, days: number) => { | |||
| export const addDays = (date: string | Date, days: number): Date => { | |||
There was a problem hiding this comment.
thanks 👍 really appreciate typings
There was a problem hiding this comment.
Types
Types
Types
Types
Types
Types
kript
kript
kript
kript
kript
kript
|
|
||
| export const trendsByCountryResponse: CallBackQueryHandlerWithCommandArgument = async ( | ||
| bot: TelegramBot, | ||
| message: TelegramBot.Message, | ||
| chatId: number, | ||
| requestedCountry?: string | undefined | ||
| requestedCountry?: string | undefined, | ||
| requestedFrequency?: Frequency | undefined |
There was a problem hiding this comment.
I know that you just followed the pattern, but I recently realized that It means
requestedFrequency might be present and might not
AND
when it presents it could be Frequency and could be undefined.
If that's interned, then go for that :)
There was a problem hiding this comment.
@danbilokha
Could you please paste what I should do with it?
Are you saying that undefined is redundant?
And I should remove it to be requestedFrequency?: Frequency
There was a problem hiding this comment.
just told you how it works so you were aware of that.
| }; | ||
|
|
||
| const capitalize = (input: string): string => |
There was a problem hiding this comment.
that function should go to utils
| @@ -190,6 +190,39 @@ export const withSingleParameterAfterCommand = ( | |||
| }; | |||
| }; | |||
|
|
|||
| export const withTwoArgumentsAfterCommand = ( | |||
There was a problem hiding this comment.
if you could integrate it with the previous approach it would be great
and it would be great if it were generic
handlerFn.call(
this,
bot,
message,
chatId,
...parameters
);
There was a problem hiding this comment.
let me know if you want to brainstorm on that together
There was a problem hiding this comment.
also, be aware, that from now these FNs have there own files (because of testing framework)
There was a problem hiding this comment.
Brainstorm IS COMMING
| ? 'Whole period' | ||
| : capitalize(requestedFrequency); | ||
|
|
||
| let model = enrichWithTitle( |
There was a problem hiding this comment.
is that really a model? Maybe a chart? Does it have a type?
There was a problem hiding this comment.
Types
Types
Types
Types
Types
Types
kript
kript
kript
kript
kript
kript
There was a problem hiding this comment.
@danbilokha
There is no need to add any type there.
TypeScript is clever enough to identify and follow the type by return type of the function
There was a problem hiding this comment.
@danbilokha try to hover your mouse on the variable. And you'll see the macig.
I know I promised to provide with a nice article on that point.
But so far I've just found. this
Proladge
force-pushed
the
improve-charts
branch
from
3b61802
to
e443e4c
Compare
| ikCbData?: string | ||
| ): Promise<TelegramBot.Message> => { | ||
| try { | ||
| const [arg1, arg2] = splitArgument(ikCbData); |
monthly, weekly and whole period charts are now available within country command inline keboard