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

#2095 Include page counts on subject browse #3850

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

symmetrically
Copy link
Contributor

@symmetrically symmetrically commented Nov 14, 2023

Closes #2095

@benwbrum
Copy link
Owner

The UI for this looks very good, but the performance is too slow for large projects.

If I log in as owner petelin then visit http://localhost:3000/cwrgm/cwrgm-rev2/subjects the page takes five minutes to load; most of which is spent executing SQL queries to find the page count for each subject.

Completed 200 OK in 318310ms (Views: 267481.2ms | ActiveRecord: 48510.1ms | Allocations: 104765563)

@benwbrum
Copy link
Owner

Functionality and performance are great. Could we modify the UI so that the number of pages is displayed next to the subject, in parentheses, but still in the grey color?

@benwbrum
Copy link
Owner

Let's abandon the column spacing and add the parenthetical page counts directly after the subject titles.

@symmetrically
Copy link
Contributor Author

@benwbrum If we abandon the column spacing, Then, it will look weird in context of UI because the Subject titles can be varied in length. Look at below screenshot.

As we are using that span in Link_to of subject title
image (98)

@saracarl
Copy link
Collaborator

Dropping in 2 screenshots of the earlier layout for comparison.
Screenshot from 2023-11-16 10-33-51
Screenshot from 2023-11-16 10-42-20

@saracarl
Copy link
Collaborator

@symmetrically could we do the link_to inside the span, with the page count immediately afterwards with only a space separating it?
Like this, more or less:

Newspapers (80)

Copy link
Collaborator

@saracarl saracarl left a comment

Choose a reason for hiding this comment

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

See earlier UI comments.

@benwbrum benwbrum merged commit faa8033 into development Nov 29, 2023
1 check passed
@benwbrum benwbrum deleted the 2095-page-count-on-subject-browse branch November 29, 2023 14:59
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.

Include page counts on subject browse
3 participants