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

Feature 2079 file display #2209

Merged
merged 104 commits into from
Apr 15, 2024
Merged

Feature 2079 file display #2209

merged 104 commits into from
Apr 15, 2024

Conversation

rushirajnenuji
Copy link
Member

status: draft PR

Add required dependencies from PackageTableView
Add functions required for the metadata view display
Fix indentation
Update DataPackageView with table render method for packageTable
Update references for PackageTableView to DataPackageView
Updated MetadataView with references for DataPackageView
Remove DownloadContentsTemplate from DataPackageView
Reference #1451 #2080 #2081
Update dataPackage to display newer file table display design
Add icons for the actions panel
Add additional classes to package table to get the styles in sync for the MetadataView
Get More Info button working in Actions panel
Add some styling changes to the Package Table View
Download Action Button init
Update download button view for actions panel
Add styling rules for rounded actions button
Moving download button to actions panel
Add more icons and buttons in the package table header
[WIP] Add expand/collapse of file icons
Update DataItem to include preview class for info icons
@rushirajnenuji rushirajnenuji marked this pull request as ready for review December 4, 2023 14:58
@robyngit
Copy link
Member

@rushirajnenuji, I've been testing out the new DataPackageView, and I must say, it looks great! The UI is going to be a big step forward. I did notice some performance issues that we might need to address. Loading data packages with a large number of files seems to take longer using the new view. Using doi:10.18739/A2SN4C with 340 files as an example:

  • The datatable in metacatui 2.27.0 takes about 9 seconds to load, whereas in feature-2079-file-display, it takes around 20-35 seconds. (Sometimes while loading, the browser slows down or the tab nearly crashes.) Any idea what might be causing this extra load time? I imagine something to do with parsing the presumably very large EML doc?
  • I also noticed that collapsing the list of files in the datatable takes about 6-10 seconds (expanding it is quick though).

See screencasts below.

For the MVP release, we could temporarily revert to the old data table view for handling very large packages. It's a bit of a workaround, considering our goal is to enhance support for larger datasets, but it might help maintain performance while we fine-tune the new feature. What do you think about this? Should we consider delaying the release to sort this out, or do you have other suggestions? Really appreciate all the effort you've put into this!!

loading-2.mov
collapse.mov

@rushirajnenuji
Copy link
Member Author

Thanks for testing the new DataPackageView and sharing your insights, @robyngit! Your feedback has been crucial for improvement.

current status: I'm fixing some algorithms introduced for these features and aim to have an update ready by tomorrow's dev call. We can also discuss the release plan on the call.

Resolve issues with slow loading of data items
- fix issues with displaying nested data packages
- display metadata object at the top of the file table
For loading spinner for package with a single object
Fix whitespace
Fix package download links
Fix More Info links
@robyngit robyngit self-requested a review January 31, 2024 18:54
@robyngit
Copy link
Member

robyngit commented Feb 1, 2024

@rushirajnenuji just added one commit with some very minor CSS/layout tweaks that:

  • keeps the table header border visible when you scroll down
  • removes the "Files in this dataset" row from the edit version of the view
  • adds a <tr> around the header elements in edit mode so it inherits the same CSS rules as the non-edit mode - specifically, keeps the header background white so you can still read the header text when you scroll down

Before:
Screenshot 2024-02-01 at 15 07 18

After:
Screenshot 2024-02-01 at 15 45 48

@robyngit
Copy link
Member

robyngit commented Feb 1, 2024

So far it's all looking great @rushirajnenuji, except that I can't get the package table to work in the dataone theme, it stays stuck on the Retrieving data set details... mode:

Screenshot 2024-02-01 at 15 47 29

I let it load for about 10 minutes and it never moved past that screen. If I switch back to the main branch then it loads as expected, so I believe it's something to do with the new changes. To reproduce the issue, just switch to the dataone theme and open any dataset in view mode. Any idea what might be causing this?

Copy link
Member Author

@rushirajnenuji rushirajnenuji left a comment

Choose a reason for hiding this comment

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

Hey @robyngit -- thank you for catching and fixing this. I tested this changes on my local instance and they look good. (sorry looks like github doesn't let me approve this, so I'm leaving a comment here)

@rushirajnenuji
Copy link
Member Author

re: dataone theme
I believed it worked in dataone theme as well, not sure if the most recent changes broke something. I'll take a look and try to add a fix for this ASAP. Thank you for catching this Robyn, I really appreciate all your time and help with getting this feature production ready.

Fixed formatting
Update dataPackage fetch
Add custom styling for DataONE theme in the Package table header
Fix issue with infinite spinners
Show error when dataPackage fetch failed
Ran tests successfully on local environment
Copy link
Member

@robyngit robyngit left a comment

Choose a reason for hiding this comment

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

Thanks to @justinkadi and Maggie for help reviewing!

src/js/collections/DataPackage.js Outdated Show resolved Hide resolved
@robyngit robyngit merged commit f80e819 into develop Apr 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants