uchicago-library / mlc_ucla

GNU General Public License v3.0
0 stars 0 forks source link

list items in search results under series #12

Closed vitorgomateus closed 1 year ago

vitorgomateus commented 1 year ago

changes should look like this: image

c-blair commented 1 year ago

I think this is very clear.

c-blair commented 1 year ago

My only suggestion is to make "items" into "item(s)" so we take care of the singular case ("one item", as opposed to "two items").

johnjung commented 1 year ago

I see we've got a conflict in static/css/mlc.css...before we merge this one, could you take a look at that and resolve the conflict? I want to be sure the right things make it to dev.

Also, in sortListOfItems(), I would have omitted parenthesis around the conditions in your if/elif statements. So rather than:

if ( isinstance(item, tuple) ): ... if( 'panopto_links' in item and len(item['panopto_links']) ): ... elif( 'has_format' in item and len(item['has_format']) ):

I would have done something more like:

if isinstance(item, tuple): ... if 'panopto_links' in item and len(item['panopto_links']): ... elif 'has_format' in item and len(item['has_format']):

Is there a reason for the extra parens here? Since we don't have to worry about the order of operations when the entire condition is wrapped in parens, can't we just omit them?

vitorgomateus commented 1 year ago

Is there a reason for the extra parens here?

Bad habits maybe. Yes we can omit.

vitorgomateus commented 1 year ago

Created a card component for items in a listing. It should look like this: image

vitorgomateus commented 1 year ago