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

Feat/page dropdown api #388

Merged
merged 44 commits into from
Mar 31, 2021
Merged

Feat/page dropdown api #388

merged 44 commits into from
Mar 31, 2021

Conversation

gweiying
Copy link
Contributor

@gweiying gweiying commented Mar 23, 2021

This PR adds the functionality to support:

  • Move
  • Delete
  • Edit settings

For folder/subfolder pages, unlinked pages and resources.

This PR is a continuation of #380, and cleans up the getPage, createPage, updatePage APIs added in #380 to resemble those in #379 for better uniformity.

Background

This PR focuses on the API integrations above, and should be reviewed with #139 on the backend. The visual appearance and UX of the Dropdown modal will be refined in a subsequent PR. There is also some buggy behavior on the Dropdown back button behavior, where clicking this series of buttons (Move To > > [folderName] > > ← Move To ) will lead to Edit Page rather than triggering the previous Move To dropdown menu as expected. This will also be fixed in a subsequent PR. Fixed in
d697a01

As part of this PR, OverviewCard, ComponentSettingsModal and CollectionPagesSection and Resources have been refactored to remove unused functionality and to use ReactQuery.

Lastly, the APIs introduced may require some cleanup as there is quite a fair bit of reused variables and flag checks. This will also be done in a subsequent PR. Fixed in 7911b8d

Details

Below, we summarise some key changes to each file.

ComponentSettingsModal

This component is now only used for settings on resource pages, so all functionality for collections and third-nav retrieval has been removed. The only API handler that runs here is Save, while the delete API handler is abstracted into CollectionPagesSection. The query call to retrieve the page details is abstracted into CollectionPagesSection so that the page details can be reused for deleting functionality.

Since we plan for the resource creation behavior to match that of page creation behavior, users will be expected to navigate to the category before creating the resource. Hence, the resource category dropdown has also been removed. See below for the new UX:
Screenshot 2021-03-24 at 6 41 37 PM

PageSettingsModal

Similar to ComponentSettingsModal, the only API handler that runs here is Save, while the delete API handler is abstracted into CollectionPagesSection. The delete, create, and update APIs from #380 have been updated. The query call to retrieve the page details is abstracted into Folders so that the page details can be reused for deleting functionality.

The UX has been updated to match that of ComponentSettingsModal. See below for the new UX:
Screenshot 2021-03-24 at 7 41 08 PM

CollectionPagesSection and Folders

The query call to retrieve the page details is added into CollectionPagesSection and Folders. The query fires when the user clicks on the ⋮ Options dropdown for any page. The new delete and move API ReactQuery handlers have been added to CollectionPagesSection and to Folders.

Note: CollectionPagesSection is actually used for all sections (unlinked pages and resources) apart from collections, so we should rename this during cleanup.

OverviewCard and FolderContent

The delete and the move handlers are abstracted into CollectionPagesSection and Folders respectively, and the logic for accessing Move To folders and subfolders have been added. This logic will be further refined based on designers input.

Changes to Resources

Since we plan for the resource creation behavior to match that of page creation behavior, users will be expected to navigate to the category before creating the resource. Hence, the option to create a new file from the Resources folder is removed, and should be replaced by the functionality to create a new resource folder #384. See below for the new UX:
Screenshot 2021-03-24 at 6 43 09 PM

Subsequent work

To highlight the changes that will be accomplished in a separate PR:

  1. Improve visual appearance and UX behavior of the Dropdown modal, waiting on designers' finalised input
  • For Move To, dropdown should start from location of file e.g. if it is a file in a subfolder, the dropdown should start at that subfolder
  • Users should not be able to move the file to the same subfolder as the file is currently in, so disabled? vs filtered out?
  1. Fix buggy behavior on the Dropdown back button behaviour with Back button (depending on UX flow, may not be necessary, waiting on designers' input) Fixed in d697a01
  2. Clean up introduced APIs for duplicated flag checking and redundant variables Fixed in 7911b8d
  3. Fix Resource ComponentSettingsModal to automatically generate file_url when resource is a file

Copy link
Contributor

@kwajiehao kwajiehao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. Additionally, there is another bug related to Move to > subfolder > Move to for unlinked pages. Steps to reproduce:

  1. Go to the Workspace layout
  2. Scroll down to the Unlinked Pages section. Click on the options button for one of the pages and click on Move to
  3. Click on the right caret button to view the selected folder's subfolders
  4. Click on the <- Move to button to go back (this brings you back to the main modal, instead of the modal screen with all the folders)
  5. Clicking on Move to again brings you directly to the subfolders view, instead of the folders view

We don't need to fix this ^ bug in this PR, just wanted to point it out so that we are aware we need to fix this in subsequent PRs

src/components/OverviewCard.jsx Outdated Show resolved Hide resolved
src/api.js Outdated Show resolved Hide resolved
Copy link
Contributor

@alexanderleegs alexanderleegs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments.

There's a bug involving the page moving modal not closing on blur - if you click Move to, then click anywhere else (even another file's options), the Move to menu persists

Also, when moving pages into subfolders, the page is automatically created as the first item in the collection.yml file.

src/api.js Outdated Show resolved Hide resolved
src/api.js Outdated Show resolved Hide resolved
@gweiying
Copy link
Contributor Author

gweiying commented Mar 29, 2021

Left some comments.

There's a bug involving the page moving modal not closing on blur - if you click Move to, then click anywhere else (even another file's options), the Move to menu persists

Also, when moving pages into subfolders, the page is automatically created as the first item in the collection.yml file.

Above was a side effect caused by not using handleBlur.

Left some comments. Additionally, there is another bug related to Move to > subfolder > Move to for unlinked pages. Steps to reproduce:

  1. Go to the Workspace layout
  2. Scroll down to the Unlinked Pages section. Click on the options button for one of the pages and click on Move to
  3. Click on the right caret button to view the selected folder's subfolders
  4. Click on the <- Move to button to go back (this brings you back to the main modal, instead of the modal screen with all the folders)
  5. Clicking on Move to again brings you directly to the subfolders view, instead of the folders view

We don't need to fix this ^ bug in this PR, just wanted to point it out so that we are aware we need to fix this in subsequent PRs

Above was caused by the queryFolderName not being cleared when a user navigates away from the dropdown modal, so on subsequent searches the subfolder view is accessed instead of the folders view. queryFolderName is now being cleared in handleBlur

Both bugs fixed in d697a01

gweiying added 21 commits March 29, 2021 22:26
This commit accomplishes several changes: 1) changes ComponentSettingModal to take pageData as input from CollectionPagesSection and removes the current API call to retrieve page data 2) changes ComponentSettingModal to use useMutation for updating the resource page settings (used for creating new page, renaming a page and changing the url of a page) 3) removes dropdown for selecting page resource category during page resource creation, so that the behavior of creating resources pages will be similar to that of folder pages, where a page can only be created after a user navigates to that location where the new file should be created
This commit refactors CollectionPagesSection with the following key changes: 1) Adding Move To functionality to Unlinked pages and restoring Move To functionality to resources 2) cleaning up  variables passed to ComponentSettingsModal since ComponentSettingsModal has been refactored 3) removing unused functionality for retrieving allCategories and improving display of pages prior to completion of loading
gweiying added 15 commits March 29, 2021 22:33
This commit pushes several changes: Firstly, UX of titles and buttons have been improved, including the display of a loading icon while query categories are being retrieved. Secondly, the bug identified earlier where the user would be redirected to the link after returning from the subfolder has been fixed. This means that the user is able to navigate between the subfolder layer and the folders layer, and return to the general dropdown menu if they would like. This fix also fixes the bug behaviors identified by Alex and Jie Hao where dropdown modals are not closed when a user clicks elsewhere on a page, and where queryFolderName is not cleared when a user navigates away from the dropdown modal
@gweiying gweiying force-pushed the feat/page-dropdown-api branch from 7b92c6c to d2fa8d0 Compare March 29, 2021 14:34
Copy link
Contributor

@alexanderleegs alexanderleegs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments.

Also one small comment - the cursor on page cards turns into a normal one instead of a pointer - we might want to change them to always show the pointer

src/layouts/Folders.jsx Outdated Show resolved Hide resolved
src/components/folders/FolderContent.jsx Outdated Show resolved Hide resolved
Copy link
Contributor

@kwajiehao kwajiehao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@gweiying gweiying requested a review from alexanderleegs March 31, 2021 07:19
Copy link
Contributor

@alexanderleegs alexanderleegs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@gweiying gweiying merged commit 8de2372 into staging Mar 31, 2021
@gweiying gweiying deleted the feat/page-dropdown-api branch March 31, 2021 09:37
@gweiying gweiying linked an issue Apr 6, 2021 that may be closed by this pull request
@gweiying gweiying linked an issue Apr 6, 2021 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

Broken Move to functionality for Resource pages Update page-moving behavior
3 participants