Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(dev): Stop code generation action #5675
feat(dev): Stop code generation action #5675
Changes from all commits
aaae062
c385dfc
e05a87a
b013042
709c0c5
644dedd
ba2db03
b791a2b
e1cbe15
ae9d16b
532e4e4
d63ceb5
f0bbd9f
e604672
675c623
2c92f5c
394c512
0743620
d16c01f
08ceb71
30735e8
047125b
08b85d1
20e5493
9597f25
e9ef770
5e194ae
aaa0a5d
8f7fa07
e63536b
326b4a9
937fb13
5f38a7c
2859246
600db42
2b9c0c0
16c587f
36c7296
af64f36
78f3968
fe0ec31
674e80a
b3dbfd1
62d6c88
b944d5a
dbd54c5
eedbda2
fe4adf7
3e355ac
9f368da
002d2c4
00a6d29
bc52d13
360cb28
aa92cc4
59ba5f9
ddfc64b
7def140
2944c4b
b08ef6e
9d91760
a12bef2
3c29518
4e51c45
3b51ec5
4ca6b1b
9608507
1391ad5
9a6a097
d7dc67e
751d87c
701792a
7528a62
fe6ea9a
c733e26
d70bb9e
b99f4db
5bfde04
14a620a
12e1376
7b7da92
3cfaf44
1fd35af
597a92d
f45a4bb
67d1958
904b4d6
14593a9
3bafa50
e8dddc1
e6c1e4f
509f835
c7bf2b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity does this mean that every single update store will need this? Why isn't the default false and then its true when we want the cancel button (which I assume is the stop button) to show?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change itself kind of scares me because we need to make sure its propagated everything and that everyone who makes changes to the updateStore knows why it exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpinkney-aws Refer to the section on MynahUI https://github.com/aws/mynah-ui/blob/0f47cdf4dcaf2574ec118cdbe7c5cdb9339d4f6d/docs/DATAMODEL.md?plain=1#L38
"In addition to the spinner, if
onStopChatResponse
is attached globally through MynahUI main class constructor properties (see Constructor properties for details) andcancelButtonWhenLoading
is not set to false specifically for that tab it will show the stop generating button too."You can sync with @dogusata about this specifically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If none of the functionalities are supporting
onStopChatResponse
why we have it attached to the mynahui instance? If some functions are using it, then this change makes sense. However the defaults can be changed for newly generated tabs. But i don't see any specific difference between giving true to the tabs which will using it or false to tabs which will not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, giving the value during the tab generation is sufficient. Also, until send a new value, the previous value will remain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpinkney-aws do we have a consensus about this specific ? Or do we need sync about it? I'm ok for now with @dogusata, no one its using it, I did safe check in all the places in this PR that we use the store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just to confirm - state of
cancelButtonWhenLoading = false
must be passed ifonStopChatResponse
was specified earlier, otherwise it defaults to true. IfonStopChatResponse
wasn't passed, then it doesn't matter.If this is correct then the current implementation is fine IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, that's the behaviour. If onStopChatResponse isn't passed, it defaults to false. Otherwise, need explicitly say false to not show it.