studio
studio copied to clipboard
fix: ask user for reload and use url in parsing
Description
- ask use for reload when spec is modified - https://github.com/asyncapi/studio/issues/393
- when url is passed in query parser uses url as
path
- https://github.com/asyncapi/studio/issues/392 - when url is passed in query, user see two buttons in editor sidebar:
save to storage
andload from storage
which one saves content from editor to the storage and switches to thelocalStorage
mode and second one loads content from localStorage and also switches to thelocalStorage
mode.
I have some problems (or rather concerns) related to the UX:
- when user pass in query
url
parameter and when click on one of two buttons described above, studio doesn't remove thaturl
parameter from href so if user will reload studio, url "logic" will be still here - should we remove url?
Any UX feedback is more than welcome :)
You can test it by passing url parametr like in example:
https://deploy-preview-394--modest-rosalind-098b67.netlify.app/?url=https://raw.githubusercontent.com/asyncapi/spec/master/examples/social-media/public-api/asyncapi.yaml
cc @derberg @mcturco
Related issue(s) Resolves #393 Resolves #392
Deploy Preview for modest-rosalind-098b67 ready!
Name | Link |
---|---|
Latest commit | 6eef8fc17a4a7752d79141a7f2f01c90c96eee1e |
Latest deploy log | https://app.netlify.com/sites/modest-rosalind-098b67/deploys/62e3a3de2e1c800009e9631e |
Deploy Preview | https://deploy-preview-394--modest-rosalind-098b67.netlify.app/ |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
ask use for reload when spec is modified - https://github.com/asyncapi/studio/issues/393
but spec is saved in local storage already, I guess on every new button release we save it and then on reload load from local storage?
better would be to enable saving of multiple AsyncAPI files in local storage, by name, and then loading by name 😉
☝🏼 this is the only reason why I would see save to storage and load from storage
making sense.
It is cool that now on import from URL you display that it was imported. And I would like to see save to localStorage
button only if I can tag the file by a name I can later use with Import from LocalStorage
.
When Importing from URL, the moment I start editing the file, label should directly change to From localStorage
and be saved there without clicking
but spec is saved in local storage already, I guess on every new button release we save it and then on reload load from local storage?
Hmm, I don't understand 😅 Browser will ask user for reload only when spec (in editor) is modified, so if you have "url" mode and if you modify editor's content you will see prompt in reloading/closing tab. Similar situation with localStorage
mode.
better would be to enable saving of multiple AsyncAPI files in local storage, by name, and then loading by name 😉
Would be, but not in this PR 😄
It is cool that now on import from URL you display that it was imported
It always indicates that you use file from url - you can check it https://studio.asyncapi.com/?url=https://raw.githubusercontent.com/asyncapi/spec/master/examples/social-media/public-api/asyncapi.yaml
When Importing from URL, the moment I start editing the file, label should directly change to From localStorage and be saved there without clicking
It's working in this way now, in deployed latest studio, but please check discussion in this issue for more details. Some users have problem with that. https://github.com/asyncapi/studio/issues/393
UX related I'd say:
- The two buttons (Save in storage and Load from storage) are simple and effective.
- I feel like I'm in control whether whatever is in my localStorage remains there, or I explicitly want to overwrite with what has been loaded from that
url
. - Tested the behavior and it works just fine: my local stuff gets saved and I don't lose it if I'm loading something (using this
url
feature).
- I feel like I'm in control whether whatever is in my localStorage remains there, or I explicitly want to overwrite with what has been loaded from that
- Once the User clicks Save in storage, I'd remove the
url
query param since it doesn't reflect the context anymore.
Regarding having multiple files locally, that could be an addition, sure, although I wouldn't stress that much with it. Maybe a polling in the community would help to see the need. To me, atm, I don't feel I'd need it.
@magicmatatjahu Thanks, Maciej, for doing this! It's really helpful.
Oh, and one more thing: I've notice that I'm being asked to confirm that I want to leave the page.
But only when I'm opening Studio with an url
query param.
That's fine, except that it always asks for this, whether I changed or not the loaded content. Again, UX related (to minimize the friction), I'd expect to be asked for this confirmation only if I changed the content (and not saved it to the localStorage, ofc).
Hi! I'm here for UX feedback :)
I agree that the two buttons are simple and effective but to me it only makes sense after I have read this PR with the context. I am wondering if for everyone else that it would be clear enough what the actions mean from those two buttons or if the user may miss it too easily?
Might I suggest that instead of adding these buttons/actions in the sidebar that we add them in the Popover
component so it would be more visible to the user? I'm thinking of verbiage along the lines of:
# You are viewing a document from another URL
Would you like to replace your last session with the contents in this document? This will erase any changes from your last session.
[Restore Previous Session] | [Update Session to Current Document]
Might need some more UX Writing thought, but this is a start to making the tool more "human-friendly" 😄
Once the User clicks Save in storage, I'd remove the url query param since it doesn't reflect the context anymore.
Agreed!
@dxps Thanks!
Oh, and one more thing: I've notice that I'm being asked to confirm that I want to leave the page. But only when I'm opening Studio with an url query param.
Hmm. Is some cases it doesn't ask in reload but in some asks, I will check that. You probably have enabled autosave so when you are in localStorage
mode then content is updated automatically so due to this fact you don't see popup in closing/reloading browser tab. I hope you understand that "feature". You should disable autosaving and then it works as should.
Once the User clicks Save in storage, I'd remove the url query param since it doesn't reflect the context anymore.
Ok, I will remove it :)
I agree that the two buttons are simple and effective but to me it only makes sense after I have read this PR with the context. I am wondering if for everyone else that it would be clear enough what the actions mean from those two buttons or if the user may miss it too easily?
Might I suggest that instead of adding these buttons/actions in the sidebar that we add them in the Popover component so it would be more visible to the user? I'm thinking of verbiage along the lines of:
@mcturco Yeah, I understand that problem, but could you precise where that popover component should be showed? Like the survey popover, in the bottom left corner or where? Or do you mean that popover component in the top right corner of editor, where you open editor menu?
@dxps Ok, now it should be fine:
- url is removed when you load or save to storage content from url
- ask popup shows when you change something in the editor's content - if it's content from url, it compares the content from url and what you have in the editor, which means that if you changed something in the editor, it will ask you about the change. When it comes to localStorage, as I described in the previous comment - it will ask you only if the content is changed in relation to localStorage - if you have auto saving then it will never ask you.
I hope it's fine :)
@magicmatatjahu Good stuff, Maciej! :man_dancing:
-
Just tested using the same deploy-preview with an url, and it's legit:
- Without changing the content, I don't get asked for a confirmation to leave the page.
- Clicking Load from storage or Save in storage it correctly loads from or saves to, respectively.
In both cases,
url
query param is removed.
-
Previously, I was referring to the fact that I was asked to confirm that I want to leave the page, only when when opening Studio with the url query param, but without even touching the content. Of course, no change was performed. But anyway, currently it seems that this has been solved (according with 1.i mentioned before).
@mcturco Yeah, I understand that problem, but could you precise where that popover component should be showed? Like the survey popover, in the bottom left corner or where? Or do you mean that popover component in the top right corner of editor, where you open editor menu?
Hmmmm, my first instinct says to put it in the bottom left like we had for the survey, but do you think that would be noticeable enough that the user knows to take action? If we think, "no" to that question, my second thought is to put it just over the document as shown in this screenshot:
What do you think?
@mcturco Thanks for response!
Hmmmm, my first instinct says to put it in the bottom left like we had for the survey, but do you think that would be noticeable enough that the user knows to take action? If we think, "no" to that question, my second thought is to put it just over the document as shown in this screenshot:
Is there a point to having this popover? It seems to me that when someone loads something from an url they would want to be able to edit the url' spec in editor without overwriting the spec from storage. Should this popover appear when hovering over something or should it always be visible?
Is there a point to having this popover? It seems to me that when someone loads something from an url they would want to be able to edit the url' spec in editor without overwriting the spec from storage. Should this popover appear when hovering over something or should it always be visible?
True, probably not the best idea to cover the spec. I guess the goal in my mind is to create an obvious "notification" that there is action required by the user on whether they want to save this new spec to local or keep what they already have and some brief explanation on what would happen if they saved the new URL data to storage.
To me, right now this notice is not super obvious, and it lacks a little bit of "warning" or context as to what would happen if the user decided to click the "save to storage" button, which could result in data loss.
@mcturco
To me, right now this notice is not super obvious, and it lacks a little bit of "warning" or context as to what would happen if the user decided to click the "save to storage" button, which could result in data loss.
Would a confirmation modal be ok?
Would a confirmation modal be ok?
Yeah that was the next thing I was thinking. Like we dull out the background and ask the user to make a selection before proceeding. But then, we get the same issue where the user can't first preview the spec file before deciding whether or not to save it. That's why I think the popover would be the best solution
@mcturco
Yeah that was the next thing I was thinking. Like we dull out the background and ask the user to make a selection before proceeding. But then, we get the same issue where the user can't first preview the spec file before deciding whether or not to save it. That's why I think the popover would be the best solution
No, no! That modal will occur only when user click button for save or load :)
Or probably best option will be to render tooltip with description what exactly save or load makes 😄
Kudos, SonarCloud Quality Gate passed!
0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells
No Coverage information
0.0% Duplication
@mcturco Ok, I added native modals for these two actions:
I have not had much time, but I will improve it when I return from vacation. What do you think about it?
Hey @magicmatatjahu I think this is a good implementation for the meantime, but would love to have a follow-up issue where we put the warnings in a modal instead of an alert window. Let me know if you want me to raise that issue!
@mcturco https://github.com/asyncapi/studio/issues/402 :)
This pull request has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.
There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
This pull request has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.
There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart: