-
Notifications
You must be signed in to change notification settings - Fork 208
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
save dropdown connectivity #1197
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1197 +/- ##
=======================================
Coverage 55.31% 55.31%
=======================================
Files 116 116
Lines 2381 2381
Branches 364 364
=======================================
Hits 1317 1317
Misses 1064 1064
|
Hi! This is great. The "save sequence" function i think doesn't really belong in this menu, but we don't have to address this right now. It's purpose was to allow people to save the entire sequence as a named sequence which would actually appear in your list of modules you can add. But it's a bit confusing to mix it with image exporting, and also it's confusing because it's not clear what it is, or how it works, and also not clear that you're not saving it in a way that /other/ people can see. I think maybe we need something like "submit sequence to gallery" or something which encourages people to develop new sequences and share them with other people. So let's just put that whole big idea aside (in a new issue?) and focus on just image exporting. How does that sound? Thanks, @aashna27 !!!! 🎉 |
The json sequence string is stored in the local storage but the the string
save sequence seems irrelevant.
…On Fri, Aug 9, 2019, 12:23 PM Jeffrey Warren ***@***.***> wrote:
Hi! This is great. The "save sequence" function i think doesn't really
belong in this menu, but we don't have to address this right now.
It's purpose was to allow people to save the entire sequence as a named
sequence which would actually appear in your list of modules you can add.
But it's a bit confusing to mix it with image exporting, and also it's
confusing because it's not clear what it is, or how it works, and also not
clear that you're not saving it in a way that /other/ people can see. I
think maybe we need something like "submit sequence to gallery" or
something which encourages people to develop new sequences and share them
with other people. So let's just put that whole big idea aside (in a new
issue?) and focus on just image exporting.
How does that sound? Thanks, @aashna27 <https://github.com/aashna27> !!!!
🎉
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1197?email_source=notifications&email_token=AGKQ7SCU5RJAGSTDOAQ4GETQDUH5ZA5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD35Y3VQ#issuecomment-519802326>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGKQ7SDPYYCXHPUYMGE2NQDQDUH5ZANCNFSM4IKA67HA>
.
|
Sequence export seems a cool idea, but the url I believe can be shared with
those specific settings applied. And we also have download gif to maybe
share the output.
On Fri, Aug 9, 2019, 12:55 PM Aashna Mittal <aashna.mittal27@gmail.com>
wrote:
… The json sequence string is stored in the local storage but the the string
save sequence seems irrelevant.
On Fri, Aug 9, 2019, 12:23 PM Jeffrey Warren ***@***.***>
wrote:
> Hi! This is great. The "save sequence" function i think doesn't really
> belong in this menu, but we don't have to address this right now.
>
> It's purpose was to allow people to save the entire sequence as a named
> sequence which would actually appear in your list of modules you can add.
>
> But it's a bit confusing to mix it with image exporting, and also it's
> confusing because it's not clear what it is, or how it works, and also not
> clear that you're not saving it in a way that /other/ people can see. I
> think maybe we need something like "submit sequence to gallery" or
> something which encourages people to develop new sequences and share them
> with other people. So let's just put that whole big idea aside (in a new
> issue?) and focus on just image exporting.
>
> How does that sound? Thanks, @aashna27 <https://github.com/aashna27>
> !!!! 🎉
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#1197?email_source=notifications&email_token=AGKQ7SCU5RJAGSTDOAQ4GETQDUH5ZA5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD35Y3VQ#issuecomment-519802326>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AGKQ7SDPYYCXHPUYMGE2NQDQDUH5ZANCNFSM4IKA67HA>
> .
>
|
Yes, let's just open a new issue (would you mind doing this?) to discuss
the sequence export possibilities. I think we'll want a modal to explain
the options, since there are pros/cons, and no matter what we'll want more
explanation shown. Thanks!
…On Fri, Aug 9, 2019 at 1:28 PM aashna27 ***@***.***> wrote:
Sequence export seems a cool idea, but the url I believe can be shared with
those specific settings applied. And we also have download gif to maybe
share the output.
On Fri, Aug 9, 2019, 12:55 PM Aashna Mittal ***@***.***>
wrote:
> The json sequence string is stored in the local storage but the the
string
> save sequence seems irrelevant.
>
> On Fri, Aug 9, 2019, 12:23 PM Jeffrey Warren ***@***.***>
> wrote:
>
>> Hi! This is great. The "save sequence" function i think doesn't really
>> belong in this menu, but we don't have to address this right now.
>>
>> It's purpose was to allow people to save the entire sequence as a named
>> sequence which would actually appear in your list of modules you can
add.
>>
>> But it's a bit confusing to mix it with image exporting, and also it's
>> confusing because it's not clear what it is, or how it works, and also
not
>> clear that you're not saving it in a way that /other/ people can see. I
>> think maybe we need something like "submit sequence to gallery" or
>> something which encourages people to develop new sequences and share
them
>> with other people. So let's just put that whole big idea aside (in a new
>> issue?) and focus on just image exporting.
>>
>> How does that sound? Thanks, @aashna27 <https://github.com/aashna27>
>> !!!! 🎉
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> <
#1197?email_source=notifications&email_token=AGKQ7SCU5RJAGSTDOAQ4GETQDUH5ZA5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD35Y3VQ#issuecomment-519802326
>,
>> or mute the thread
>> <
https://github.com/notifications/unsubscribe-auth/AGKQ7SDPYYCXHPUYMGE2NQDQDUH5ZANCNFSM4IKA67HA
>
>> .
>>
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#1197?email_source=notifications&email_token=AAAF6JYV6EPN4LN77K2L3H3QDUL6LA5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD353G4Q#issuecomment-519811954>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAF6JYNMBRNLK3BVC7CKP3QDUL6LANCNFSM4IKA67HA>
.
|
i have opened issue and what about merging this one, I ll remove the "save string string" from the dropdown. |
ok, that sounds good, then are we ready to merge here then? Thanks!!!!
…On Fri, Aug 9, 2019 at 11:05 AM aashna27 ***@***.***> wrote:
i have opened issue and what about merging this one, I ll remove the "save
string string" from the dropdown.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#1197?email_source=notifications&email_token=AAAF6J4BTUZYXMPKNIGIB6LQDWBTJA5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3656NY#issuecomment-519954231>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAF6J4Y2LWMNRRVKM73ECTQDWBTJANCNFSM4IKA67HA>
.
|
Yes i think it's good to go
On Wed, Aug 14, 2019, 9:34 PM Jeffrey Warren <notifications@github.com>
wrote:
… ok, that sounds good, then are we ready to merge here then? Thanks!!!!
On Fri, Aug 9, 2019 at 11:05 AM aashna27 ***@***.***> wrote:
> i have opened issue and what about merging this one, I ll remove the
"save
> string string" from the dropdown.
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <
#1197?email_source=notifications&email_token=AAAF6J4BTUZYXMPKNIGIB6LQDWBTJA5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3656NY#issuecomment-519954231
>,
> or mute the thread
> <
https://github.com/notifications/unsubscribe-auth/AAAF6J4Y2LWMNRRVKM73ECTQDWBTJANCNFSM4IKA67HA
>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1197?email_source=notifications&email_token=AGKQ7SBS5A7EK6VT7KXR4FTQEQUH7A5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4JI76Q#issuecomment-521310202>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGKQ7SDIMNL7BVU2JZ2H553QEQUH7ANCNFSM4IKA67HA>
.
|
Hi, looks like it needed an update, so i triggered it to sync. Would you consider writing a UI test for this functionality to preserve it? Thank you @aashna27 !!! |
The ui testing is yet to be figured out. So maybe if before that we can
merge this.
…On Wed, Aug 14, 2019, 10:17 PM Jeffrey Warren ***@***.***> wrote:
Hi, looks like it needed an update, so i triggered it to sync. Would you
consider writing a UI test for this functionality to preserve it? Thank you
@aashna27 <https://github.com/aashna27> !!!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1197?email_source=notifications&email_token=AGKQ7SAZBMRVGOW5A7ODLFTQEQZJLA5CNFSM4IKA67HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4JM2OY#issuecomment-521325883>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGKQ7SFIW7WIXMZBNOLJNKDQEQZJLANCNFSM4IKA67HA>
.
|
OK, sounds good. Thanks! |
* dropdown connectivity for 2 options * comment added * save gif corrected and preview gif refactored
Fixes #1192
Right now I have figured out for only 3 dropdown option->
npm test
@publiclab/is-reviewers
for help, in a comment below