NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 27 forks source link

Refactor Portals list view for use outside of Settings menu #1708

Closed vchendrix closed 3 years ago

vchendrix commented 3 years ago

Describe the feature you'd like In order to help make public portals more easily discoverable. ESS-DIVE would like to make access to the portal list from a main menu on the website. This will require the portal list view to be redesigned to facililate portal discovery and also for portal editors to access their managed portals

Is your feature request related to a problem? Please describe. The problem is that once a portal is published there is no easy way to "discover" portals. The current design of the portal list view does not take into account public portals.

Additional context Here is an early mockup of the refactored portal portals

Refactor the portals list view to based on the mockups. There will be two lists 1) My portals - those portals that the user can edit 2) All portals - all publicly viewable portals and view only portals (private but shared with me)

Other Features

1) Each portal list will have paging. The default page size will be fixed 2) The My portals section will have an edit button only 3) If a user does not have permissions on a non-public portal, this section will not show up.

Questions

vchendrix commented 3 years ago

@csjx @gothub @laurenwalker I updated the ticket with the new mock up and details some requirements. I added a Questions section for things we didn't discuss in our meeting.

vchendrix commented 3 years ago

@gothub I just noticed today that if a user does not have any portals in their list, they don't get a new button to create portals. Let me know if you want me to log a separate bug for this.

Screen Shot 2021-04-22 at 8 27 17 AM
gothub commented 3 years ago

@vchendrix This functionality is now available in the feature-#1708-portals-list branch.

The 'portals' menu item is enabled with the config parameter:

enablePortalsList: true
mbjones commented 3 years ago

@gothub could you please post in this issue a screenshot of the developed feature with representative portals in the listing so we can all see what the implementation looks like? Thanks. It might also be nice to be able to preview it on a development server somewhere so people can try it out.

gothub commented 3 years ago

@mbjones here are screenshots of the portals list in the default view.. As I explained to @vchendrix, this demo is using daa from arcticdata.io, as the ESS-DIVE MN CORS disallows connections from my local setup. Also, Val had mentioned that some from ESS-DIVE will evaluate the modifications in the git branch on one of their own systems. The first screenshot shows the view when I'm not logged in, the next when I am, as a user in the arctic-data-admin group that can edit all portals.

All-portals My-portals
mbjones commented 3 years ago

Nice @gothub they look great. Let's plan to discuss on our Thursday Dev call.

gothub commented 3 years ago

@laurenwalker I pushed changes that display the proposed portals list result. We had discussed having two DataCatalogView instances created from PortalSearchView, with one being the 'My Portals' version with the permissions filter (for the Solr result) and another DataCatalogView with the inverse permissions filter for the "All Portals" section.

After giving that a try, the result is two DataCatalogViews on the same page, with the resulting problem of the second view having several duplicate id HTML elements, which makes an "invalid" document in regards to the W3C standard, and can result in JQuery, CSS only finding the first id instance.

Note that the checked in code for 'PortalSearchView.js' does not demonstrate this behaviour by creating a second DataCatalogWIthFilters view (with different filters for the second view), as I didn't see the point in checking in code that doesn't work.

So, I'm wondering if we need to take a step back and reconsider the proposed design.

vchendrix commented 3 years ago

enablePortalsList: true

@gothub @laurenwalker is there a workaround for this in the 2.14.0 release? We are having a community meeting this Monday and Tuesday. One of the tutorials is how to create portals and if the new button doesn't show up user's can't create portals

gothub commented 3 years ago

@laurenwalker @csjx the 43c8f0f5debc69a786be00d6dc8ad37237097977 checkin includes the changes we discussed on 5/20 at the NCEAS dev meeting. Please note:

gothub commented 3 years ago

OK, since ESS-DIVE will be using a 2.14.x release, the portals list code has been checked into the feature-#1708-portals-search branch, which was branched from dev-2.14.

laurenwalker commented 3 years ago

I reviewed and made some changes to this feature in the feature-%231708-portals-list branch. I deployed the default theme to https://fancy-vulture.nceas.ucsb.edu/portals/ so others can view a live demo right now (with the default theme. Make sure you are logged in to see the "My Portals" section).

Once @gothub @vchendrix and others have reviewed this feature, we can include it in the 2.16.0 release.

helbashandy commented 3 years ago

Hi @laurenwalker - We have reviewed the list and it's looking great. We only have two minor issues to suggest.

1 - For portals that do not have logos, it would be ideal to have some sort of a placeholder image so that it would have a more organized look in a list of portals that only some have a logo, and also remind/encourage users to add a logo for their portal.

Screen Shot 2021-06-24 at 2 13 54 PM

2 - In the public view, portals are listed without a title or a label. I think it would be ideal to just add a label such as All portals before the portals list.

Screen Shot 2021-06-24 at 2 13 39 PM

gothub commented 3 years ago

@helbashandy good idea regarding a placeholder image.

Regarding the second point, when I'm not logged in and accessing https://fancy-vulture.nceas.ucsb.edu/portals/ the page that is displayed is:

Screen Shot 2021-06-24 at 2 34 25 PM

Are you working from the branch mentioned above?

helbashandy commented 3 years ago

Hi @gothub - Yes, I am working on the feature-#1708-portals-list branch mentioned above. Is there another branch?

I'm taking a look at the code and it seems like the All portals keyword is added on this line which is enclosed under a conditional loggedIn check block here.

To double check in case an element is hidden, I also searched through the rendered HTML element for All portals but didn't find it.

Is there any other place in the code where (maybe HTML that needs merging on our end) the All Portals term is added?

Otherwise, Is it possible that the deployed version is different than the latest commit on the branch?

laurenwalker commented 3 years ago

Yes, it's likely that the deployed version is a few commits behind.

csjx commented 3 years ago

I agree with @helbashandy's comments above. The overall list looks good on a smaller XGA screen (1024x768), but I think we have a bit of a layout issue going on with regard to left or center justification even on a standard HD screen size (1920x1080), and it is somewhat blatantly askew on a 4K monitor. When not logged in, we see:

full-hd

The list is left justified, but the pager is centered, and so there's a huge amount of whitespace to the right. It's slightly better when logged in because of the right-justified edit buttons:

full-hd-logged-in

But still, it is awkwardly laid out, and to me reduces readability. I'm wondering if we want to center justify everything into a div that is more like 50% of the total width on wider screens so that there is a little more wrapping and the titles are more readable.

laurenwalker commented 3 years ago

Yes, it's likely that the deployed version is a few commits behind.

@gothub - I had these changes locally, not committed, so I just pushed them to the feature branch now.

gothub commented 3 years ago

@csjx regarding the portals list centering suggestion, it's trivial to add the CSS:

#portals-list-container {
    width: 75%;
    margin: auto;
}

50% looks cramped both on laptop and large display, but the CSS change doesn't quite work either. @laurenwalker is there a way to have the width 100% for smaller displays, but then something different (75%?) for larger displays. What do you recommend?

csjx commented 3 years ago

We can use media queries to size the content based on the viewport @gothub . See https://developer.mozilla.org/en-US/docs/Web/CSS/Media_Queries/Using_media_queries

gothub commented 3 years ago

@laurenwalker The portals list now resizes for large displays. The last item, I believe is setting a default background to display if a logo is not provided. However, setting a default background doesn't look great for logos that don't fill the image area, or possibly have transparency set. A style could be set inline after testing in PortalSearchView, but I'm thinking that we don't like applying styles that way. Is there a way to set a default light grey background without getting this type of result:

Screen Shot 2021-07-06 at 1 22 08 PM
gothub commented 3 years ago

@laurenwalker changes in the feature-#1708-portals-list branch are now ready for release, pending your review. At the ESS-DIVE tech meeting yesterday, @val indicated that the current portals lists fulfills their needs for now, and any additional modifications (the least of which is the logo image background) can wait for a future release.

laurenwalker commented 3 years ago

Great, thanks! I will get it merged to develop today and I am going to try to tag the release today or tomorrow.