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

[issue] let edit feature fit figma design & UX smooth #2273

Closed
sync-by-unito bot opened this issue Nov 3, 2022 · 47 comments
Closed

[issue] let edit feature fit figma design & UX smooth #2273

sync-by-unito bot opened this issue Nov 3, 2022 · 47 comments

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Nov 3, 2022

Updated: order to fit figma design for asset page edit feature (see comment or sub-task)

  1. Temporarily remove "edit" on options list now.
  2. add back this edit totally fit figma design, like this flow → could add them on future sprint
  3. when Bump protractor from 5.4.4 to 7.0.0 #2 finish, remove current edit feature on iframe

Reproduce steps

  1. choose an asset, press upper right button
  2. press edit
  3. type content, press "ok"
  4. the caption doesn't display the new content, need re-enter could display it.

User story

As a capture app user, I want the edited content could be display immediately so that I can confirm the caption is.

┆Issue is synchronized with this Asana task by Unito
┆Created By: Kenny Hung
┆Link To Task: https://app.asana.com/0/1201016280880500/1203292734517486

@sync-by-unito sync-by-unito bot closed this as completed Nov 4, 2022
@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 4, 2022

➤ Sam commented:

Here is PR ( #2278 ) (claap included)

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 4, 2022

➤ Sam commented:

Kenny Hung, when I was doing this task I also realized that I can improve explore tab reload.

Here is the reload explore tab UX before ( https://i.imgur.com/gafQtkI.mp4 ) and after ( https://i.imgur.com/ER1yXWD.mp4 ).

If you want I can

  • create a ticket something like (better UX for reload explore tab)
  • add as a dependency for v221108
  • and submit the PR code

Or we can do it next sprint let me know what you think

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 7, 2022

➤ Kenny Hung commented:

Sam (cc Tammy Yang )You mean you could improve the explore tab reload performance further?

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 7, 2022

➤ Tammy Yang commented:

Sam yes, please do. Performance is the biggest priority in general, so let's always try to include the fix in the latest confident sprint

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 7, 2022

➤ Kenny Hung commented:

SamTammy Yang I have opened this card improve explore tab reload ( https://app.asana.com/0/1201016280880500/1203324312350000/f )and add as depoendency for v221108. Thanks.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 9, 2022

➤ Bofu Chen commented:

Sam Can you remind me that why do we need this PR: #2278 (review) ( #2278 (review) )

Currently, to update the caption, I can

  1. Click an asset in the profile tab
  2. Click the asset's caption directly (not click the settings button on the top-right corner)
  3. Edit the caption

SamTammy Yang Is it better if we

  1. remove the "Edit" option, or
  2. make it have the same behavior as clicking the caption

@sync-by-unito sync-by-unito bot reopened this Nov 9, 2022
@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 9, 2022

➤ Sam commented:

Bofu Chen without this commit if you update caption the changes will not be reflected so you need to go back and open details page to see updated caption

So that pr reloads iframe in details page when caption is updated.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 9, 2022

➤ Bofu Chen commented:

If I updated the caption by clicking it directly, I can update the caption smoothly.

If I updated the caption by clicking the "Edit" option, I can reproduce this issue. That's why I suggest to remove the "Edit" option or leverage the existing mechanism.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

➤ Sam commented:

Kenny Hung, did you know that we can update the caption from the iframe?
Here is the demo ( https://i.imgur.com/uLkMtKd.mp4 ), I think updating the caption from the iframe is better because it works well with the iframe.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

➤ Kenny Hung commented:

I know that, but from figma design, it should edit on options button. It's my proposal.

  1. Temporarily remove "edit" on options list now.
  2. add back this edit totally fit figma design, like this flow ( https://app.claap.io/numbers-protocol/edit-feature-on-figma-design-c-O35CsUM4Uy-Mv2bQgEt4GjX ) → could add them on future sprint
  3. when Bump protractor from 5.4.4 to 7.0.0 #2 finish, remove current edit feature on iframe

It could have the close UX in a short term & fit the figma design in the future.
How about you? Tammy YangSamEthan Wu

@sync-by-unito sync-by-unito bot changed the title [issue] edit need re-enter the asset page could show the change. [issue] let edit feature fit figma design & UX smooth Nov 10, 2022
@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

➤ Tammy Yang commented:

Kenny Hung agreed, let's do that.
Make sure edit in iframe match the Figma design.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

➤ Ethan Wu commented:

so i think the challenge here is that for the asset page in profile tab we leverage both ionic and iframe elements

asset page (iframe)

top tool bar (ionic)

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

➤ Kenny Hung commented:

rename this asana card.
Sam because the 0.69.1 is release to QA, I will add the sub-task to track and add it ( https://app.asana.com/0/0/1203342620207712/f ) to next Tuesday release.
Ethan Wu yes, so I think we need more time to figure it out.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

➤ Sam commented:

Kenny Hung, can I get the flow URL, please. from this [claap ( https://app.claap.io/numbers-protocol/edit-feature-on-figma-design-c-O35CsUM4Uy-Mv2bQgEt4GjX )]

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 10, 2022

➤ Sam commented:

Ethan Wu here is the proposal ( https://app.claap.io/numbers-protocol/ethan-proposal-on-how-to-implement-edit-caption-c-O35CsUM4Uy-jwV7mbmEbft_ ) on how to implement edit caption and fit UX design.

From the bubble side, you need to send events like "cancel" or "save".

  • For cancel button, you can use this script

    • window.parent.postMessage('edit-caption-cancel', '*');
  • For save caption button you can use this script

    • window.parent.postMessage('edit-caption-save', '*');

And from the capture app, I receive those events and do some logic such as

  • navigate back.
  • navigate back and reload the iframe on the details page to reflect caption changes.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

Sam for the save event will ionic do the capture edit or will we call it from the bubble side.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

Bubble side will do edit and saving.
Ionic side just receives events indicating that caption was saved or canceled

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

Sam do we need to change the page layout?

according to the flow that kenny provided the "save" button is in the top bar and i believe that is an ionic component

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

Ethan Wu yes all ui components are implemented in bubble side

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

Sam so when we enter the edit page the back button and save buttons will all be bubble?

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

All will be bubble side. From ionic side I only provide a screen that will host iframe

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

Sam so there will be no ionic components in the screen view?

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

Ethan Wu correct. No ionic components. All in bubble: back button, save button and form inputs.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

ok so which iframe url are you going to pass on this screen?

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

Edit caption screen will only host iframe and pass capture id to iframe url

And listen for postMessages() from bubble side:
Edit-caption-save edit-caption-cancel

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

Sam ok got it. i will work on it

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

Ethan Wu i assume ../edit-caption/?cid=xxxx

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

Sam yeah something like that. i just need to understand whether or not i needed to create another bubble page and it looks like i need to.

before everything was done in the asset page

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

Ethan Wu, I thought you will copy the existing edit caption in a new page😅.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Ethan Wu commented:

Sam yeah it won't be difficult to do. i will just copy over the asset page and strip out the elements that aren't related to edit capture and fix up the top bar. no worries haha

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 11, 2022

➤ Sam commented:

Ethan Wu, okay thank you 🙏. Please let me know if I can assist.

Once bubble UI implementation is done I need one more thing

  • When back button is clicked on the bubble side please run this JS

    • window.parent.postMessage('edit-caption-cancel', '*');
  • When save button is clicked please run this js

    • window.parent.postMessage('edit-caption-save', '*');

And from ionic side I will do the rest. So if its edit-caption-cancel I just navigate back. But if it's edit-caption-save I will navigate back and reload iframe in details page.

P.S: just to clarify caption save still happens on bubble side.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 14, 2022

➤ Ethan Wu commented:

Sam the page is ready. you can test with this iframe url link:

https://captureappiframe.numbersprotocol.io/version-v221115/edit/?from=mycapture ( https://captureappiframe.numbersprotocol.io/version-v221115/edit/?from=mycapture )&nid=&token=&refresh_token=

@sync-by-unito sync-by-unito bot closed this as completed Nov 14, 2022
@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 14, 2022

➤ Sam commented:

here is the PR ( #2310 )

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 16, 2022

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 16, 2022

➤ Ethan Wu commented:

the above is the current flow implemented

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Kenny Hung commented:

Sam (cc Ethan Wu) There are some questions about edit feature on asset page options.

  1. When I use iframe qa version, it could display, but I tried on live version, it will display as this attachment. Does this feature needs iframe deploy iframe-v221115?
  2. When I modified through edit(options), after I press save & back to asset page, the option still doesn't change. It still needs to leave & come back this page, then could see the caption changed.
  3. iframe edit still exists.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Sam commented:

Kenny Hung let me check

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Ethan Wu commented:

Kenny Hung it needs to be deployed before it can be used

the edit page only exists in qa-release branch

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Kenny Hung commented:

Thanks Ethan Wu clarify.
Sam I tried it on 0.69.2 x iframe qa, found #2.
Remind: only when #2 fixed, we could implement #3.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Ethan Wu commented:

Sam its not deployed yet

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Kenny Hung commented:

Ethan WuSam
Just confirm, when v221115 deployed, #2 could be fixed?

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Sam commented:

Kenny Hung, I believe #2 can be fixed when captureirame is deployed (Ethan Wu mentioned it's not deployed yet).

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Ethan Wu commented:

Kenny Hung yes you should not test the edit page until the v221115 is deployed

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Nov 22, 2022

➤ Sam commented:

I believe there are no extra actions needed from the ionic side because it's working when iframe is qa-branch. Just need to wait when iframe prod branch is deployed and test again.

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

No branches or pull requests

0 participants