Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #909 +/- ##
==========================================
+ Coverage 47.75% 47.91% +0.16%
==========================================
Files 350 350
Lines 11313 11321 +8
Branches 1893 1894 +1
==========================================
+ Hits 5403 5425 +22
+ Misses 5717 5708 -9
+ Partials 193 188 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2f8a424 to
d2787b0
Compare
cecilia-donnelly
left a comment
There was a problem hiding this comment.
Testing notes:
- Need a test account with multiple archives
- Check both restricted links and unrestricted
It appears that this only applies to restricted links. I didn't see a way to add an unrestricted shared item (single record) to my archive, so there wasn't a "switch archives" flow either. Is that expected?
I think this is an error, but maybe not:
The "back" button closed the modal (correct) but also accepted the share and took me into my workspace. That was unexpected for me because I thought it would behave more like "cancel," but it's really a product question because I had at that point already clicked "Accept Share," so it might be fine.
Overall the code makes sense and this is a definite improvement, thank you @aasandei-vsp!
I'll approve and ask @omnignorant to address the question about the "back" button.
Thank you @cecilia-donnelly These are great points, I copied the testing notes to the PRs description. Related to the unlisted share, the story I used for implementation mentioned that we will not be adding them to the user's archive. It seems though that mobile does that, so we will implement it too: https://permanent.atlassian.net/browse/PER-10443 The back button functionality is a bit weird, indeed, but as you mentioned, we do need a business decision and some design for that. I do have a story for improvements, so I'll add this as well: https://permanent.atlassian.net/browse/PER-10385 |
omnignorant
left a comment
There was a problem hiding this comment.
I have some important changes that need to be made. They should be done in steps. The first step should be completed before this PR is approved because it is easy, fixes the "buggy" back behavior and meets all the requirements for this flow.
The second step should be a separate ticket because I think it is more complex.
STEP 1:
- Change archive selection modal title text from "Select archive to switch to"
- New modal title text "Select archive to request access"
- Change "Back" button text to "Current Archive"
These changes are more accurate to the current functionality. The new button text will now yield the expected result: when you click "Current Archive" in the "Select archive to request access" the share request will be made from the current archive.
STEP 2:
- Keep the title text from Step 1 as-is.
- Add a "Cancel" button or hyperlink under the buttons to terminate the request flow and return to the preview screen with "Request Access" CTA.
- Remove "Current Archive" button from Step 1 above.
- Add the current archive to the list of archives to select from so that when chosen, the request is made for the current archive, e.g. what the back button was doing before changes in step 1.
Now we introduce a real cancel button and make the current archive an option that is presented consistently with the other archives.
IF it would be as fast to do Step 2 as it would be to do Step 1, then we should just do Step 2 before approving this PR. E.g. it is my assumption that doing the items in Step 1 may add a day or two but the items in Step 2 will add at least a sprint.
CC @tiberiuVSP because I think we might need to update designs to match, though I don't think we need new designs to complete the steps.
@omnignorant I will fix STEP 1 in this PR, as it is very fast, but for STEP 2 I have created a different task where I have asked @tiberiuVSP to maybe have a look at the design. |
f9c0368 to
b837977
Compare
b837977 to
f233fa0
Compare
omnignorant
left a comment
There was a problem hiding this comment.
This is working great! Well done!!! I'm so glad.
…ton is clicked When sharing a restricted record or folder, the user needs to request access and choose which archive the record or folder will be added to. After the selection, the confirmation prompt would close, but not the archive switcher modal. For this, the dialogref class had to be added to the component and its close method called.
The back button on the archive switcher was not doing anything. Now it will close the archive switcher modal, no matter on what screen the modal will be overlayed. Issue: https://permanent.atlassian.net/browse/PER-10322
The prompt component should be shown over the main component, a dialog and the opaque layover. This is why it needs a z index that will be higher than any other. Issue: PER-10322 Refactor choose from multiple archives share link
When a share is restricted, the user either needs to accept the share or request access. In the process, the user can also choose to which archive the share will be added. This commit adds the share to the correct archive. Issue: PER-10322
After creating a new archive, switching to it was yielding in an error because the newly created archive needed to be first retrieved. Issue: PER-10322
…ionality The archive switcher title will now reflect the fact that once you choose that archive, you will be on your path to accept the share there. Clicking the Current archive button, it will add the share to the currently selected archive and redirect the user to the shared workspace of that archive. ISSUE: PER-10322
f233fa0 to
dc75027
Compare
Testing notes:
Need a test account with multiple archives
Check both restricted links and unrestricted
This only applies to restricted shares. We will address the multiple archive choice for unlisted shares when we implement the functionality for adding them to the user's archive.
Issue: https://permanent.atlassian.net/browse/PER-10322