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

Toward replicating fact architecture for searches #120

Closed
wants to merge 1 commit into from

Conversation

matt-bernhardt
Copy link
Member

** Why are these changes being introduced:

  • The TIMDEX API responses can be quite slow at times, and the response of the application can be frustrating to users when that happens. The browsers' page-loading indications can be subtle, and users now expect more prominent confirmation that their request has been received.

** Relevant ticket(s):

** How does this address that need:

THIS IS A WORK IN PROGRESS, NOT A FINISHED COMMIT.

  • This implements a loading-status message by replacing our search architecture (directly loading results and including them in the first meaningful response to the user) with the approach taken in our fact panels - using a data controller for the search results, while moving the search results to the static page controller.

  • The tests are broken, because this is only a partial implementation.

** Document any side effects to this change:

  • I am committing these changes and sharing now, because I want a point of comparison with an alternative approach - using turbo frames. The point I've reached here is to now need to replicate the request validation in the static page controller (or to abstract out the validation from the search controller to something called both by search and static controllers). This feels icky, so we are going to see if turbo feels better.

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

YES | NO

Includes new or updated dependencies?

YES | NO

@mitlib mitlib temporarily deployed to timdex-ui-pi-gdt-143-st-v8bhhd February 23, 2024 19:02 Inactive
** Why are these changes being introduced:

* The TIMDEX API responses can be quite slow at times, and the response
  of the application can be frustrating to users when that happens. The
  browsers' page-loading indications can be subtle, and users now
  expect more prominent confirmation that their request has been
  received.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/gdt-143

** How does this address that need:

THIS IS A WORK IN PROGRESS, NOT A FINISHED COMMIT.

* This implements a loading-status message by replacing our search
  architecture (directly loading results and including them in the first
  meaningful response to the user) with the approach taken in our fact
  panels - using a data controller for the search results, while moving
  the search results to the static page controller.

* The tests are broken, because this is only a partial implementation.

** Document any side effects to this change:

* I am committing these changes and sharing now, because I want a point
  of comparison with an alternative approach - using turbo frames. The
  point I've reached here is to now need to replicate the request
  validation in the static page controller (or to abstract out the
  validation from the search controller to something called both by
  search and static controllers). This feels icky, so we are going to
  see if turbo feels better.
@mitlib mitlib temporarily deployed to timdex-ui-pi-gdt-143-st-v8bhhd February 23, 2024 20:11 Inactive
@matt-bernhardt
Copy link
Member Author

This change was supplanted by #122

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.

2 participants