Skip to content

Manually selecting a reconciliation match returns the grid to page 1. #33

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

Closed
tfmorris opened this issue Oct 15, 2012 · 25 comments · Fixed by #6546
Closed

Manually selecting a reconciliation match returns the grid to page 1. #33

tfmorris opened this issue Oct 15, 2012 · 25 comments · Fixed by #6546
Assignees
Labels
grid pagination / scrolling About scrolling through the rows via pagination or potentially seamless scrolling imported from old code repo Issue imported from Google Code in 2010 Module: Frontend These issues involve working on HTML, CSS, and JavaScript code that affects the user interface. Priority: Medium Represents important issues that need to be addressed but are not urgent reconciliation Related to the reconciliation operations and other features Theme: UX/Usability Focuses on issues related to improving the overall user experience and interaction flow. Type: Bug Issues related to software defects or unexpected behavior, which require resolution. Type: Feature Request Identifies requests for new features or enhancements. These involve proposing new improvements.
Milestone

Comments

@tfmorris
Copy link
Member

tfmorris commented Oct 15, 2012

Original author: staringmonkey (May 14, 2010 14:09:53)

What steps will reproduce the problem?

  1. From the header of a data column select Reconcile -> Start Reconciling.
  2. Select and appropriate topic and click Start Reconciling.
  3. Wait for the reconcile to complete.
  4. Click the "next page" link.
  5. Find a cell that which has several possible reconciliation matches and
    click the check-mark or double-check-mark image.

What is the expected output? What do you see instead?

Reconciliation is completed correctly, however, grid is returned to page
one. This makes it nearly impossible to complete manual reconciliation for
large datasets.

What version of the product are you using? On what operating system?

SVN HEAD on OSX.

Please provide any additional information below.

Original issue: http://code.google.com/p/google-refine/issues/detail?id=33

@tfmorris
Copy link
Member Author

From dfhu...@gmail.com on May 14, 2010 16:32:32:
For now, what you can do is as follows:

  • select None in the Judgment facet to see only those rows that haven't been matched.
    Or if the facet isn't there, it's available in the column's menu Reconcile -> Facets ->
    By Judgment.
  • just keep on the first page and process the first row. Once you manually match it, it
    goes away (since its judgment is no longer None).

@tfmorris
Copy link
Member Author

From staringmonkey on May 14, 2010 16:37:58:
@dfhuynh That's an excellent stopgap solution--thanks for suggesting it!

@wetneb wetneb added the reconciliation Related to the reconciliation operations and other features label Aug 2, 2017
@thadguidry
Copy link
Member

@wetneb Does this happen on your working branch any longer with Wikidata new parts ? If not, then let's close this out noting so.

@wetneb
Copy link
Member

wetneb commented Nov 8, 2017

@thadguidry yes this is unfortunately still current (it is not specific to freebase).

@ettorerizza
Copy link
Member

Here is a visual example. As you can see, I'm on pages 91-100. But as soon as I make a reconciliation, I come back to page 1.

screencast2

@wetneb wetneb changed the title Manually selecting a Freebase reconciliation match returns the grid to page 1. Manually selecting a reconciliation match returns the grid to page 1. Nov 8, 2017
@ostephens
Copy link
Member

See also #569 - similar issue (user is returned to start (row 1) of project), but in relation to a different action ("Apply to all identical cells" on manual cell edit)

@thadguidry thadguidry added Priority: Medium Represents important issues that need to be addressed but are not urgent and removed Priority: Low Indicates less critical issues that can be dealt with at a later stage labels Nov 8, 2017
@thadguidry thadguidry added the Type: Feature Request Identifies requests for new features or enhancements. These involve proposing new improvements. label Mar 30, 2019
@thadguidry
Copy link
Member

@wetneb @ostephens We'll use this #33 as the primary issue for the annoyance by users. I think we should open a new issue that captures the Enhancement introduction of a new "Reconcile Mode" that doesn't flip back to row 1, but keeps the Reconcile -> Facets -> By Judgement enabled with selected as "None" as the default. And that facet can't go away until they unclick "Reconcile Mode". We can also add other features to this "Reconcile Mode" as needed (some default behaviors that get enabled or some special handling).

  1. They click on a button to start "Reconcile Mode". (perhaps next to records link)
  2. User is given a message "Reconcile Mode is now ON"
  3. The By Judgement facet then shows a count of all rows selected in "none" and 0 "matched" rows.
  4. User begins reconciling with dialogs.
  5. By Judgement facet stays enabled and cannot be removed.
  6. User wants to stop "Reconcile Mode" and clicks some button and is given a message "Reconcile Mode is now OFF".
  7. By Judgement facet goes away.

Thoughts?

@wetneb
Copy link
Member

wetneb commented Mar 30, 2019

I am not sure such a "Reconcile Mode" would offer much value over the current workaround: that is already what the facet does. I was thinking of a dedicated UI for matching (separate from the grid view) which would be optimized for that.

@thadguidry
Copy link
Member

thadguidry commented Mar 30, 2019

Then how do you propose we make this issue go away and not so frustrating for users? Oh by that dedicated UI ?

@wetneb
Copy link
Member

wetneb commented Mar 30, 2019

Yes. Or for this issue only, it could also be done by simply making sure the page is preserved when we apply an operation, but it might not be worth investing too much effort in that if we want to migrate to a scrollable grid in the end.

@thadguidry
Copy link
Member

Agreed. I'd rather us focus on a dedicated UI with a scrollable grid for Reconciling.

@wetneb
Copy link
Member

wetneb commented Aug 6, 2020

Document summarizing the approaches @lisa761 and I have been thinking about to solve this issue:
https://docs.google.com/document/d/1zw_Nj0O5KNRmEIPwc6gNTF4tdhSYWDEOVYPAr-F5gi4/edit#

@tfmorris
Copy link
Member Author

tfmorris commented Aug 7, 2020

Thanks for summarizing the options. From a user's point of view, I think their ideal outcome is that screen gets re-rendered in a minimal fashion, ie any updates that are required get made in situ with the rest of the grid being unchanged and fixed in the same position. Any rows which are no longer visible should smoothly "melt" away with their neighbors being smoothly scrolled into place.

Obviously there are going to be pathological cases where the entire screen ends up getting repainted anyway, but, ideally, for smaller updates, the user should be able to visually track what's happening and retain their context. Of course, the benefit of the "ideal" solution needs to be weighed against the implementation cost.

@fsteeg
Copy link
Member

fsteeg commented Jul 22, 2021

At least in 3.4.1, master and 4.0, the original issue only affects the dataset-wide operations (Match All Identical Cells / Search for match -> Match other cells with same content). The single-cell operations (Match this Cell / Search for match -> Match this cell only) remain on the current page. (We might want to update the issue title for that.)

Based on that, maybe it would be an improvement to flip the default of Match other cells with same content to Match this cell only. This would provide the user a more basic, but working default workflow. I'd also imagine that with a conscious decision to match all cells, the jumping behavior would appear less buggy and more like a missing feature ('right, I changed the entire dataset, so now I'm at the beginning again'). It might also make sense to flip the visual order of Match other cells with same content / Match this cell only to correspond to the ordering of the Match this Cell / Match All Identical Cells buttons.

@wetneb
Copy link
Member

wetneb commented Nov 3, 2022

I would like to propose a solution to this issue based on the new architecture developed in the 4.0 branch. I have not implemented it yet, but I want to discuss the implications here first.

For technical reasons, the API offered by the backend has changed in the new architecture. To fetch a given page of rows, the frontend had to provide three things: the configuration of the engine (row/records mode and facets), the number of rows to skip before the beginning of the page, and the size of the page itself. With the new architecture, instead of providing the number of rows to skip before the page, we provide the row number of the first row to start displaying in the page. That is not necessarily the same thing if some rows have been filtered out by facets.

This internal API change is an opportunity to change the user interface too. Instead of letting the user input a page number, this new API invites us to let the user provide the row number of the first row to show. On its own, I do feel like (at least hope) this is not harming the usability much. But the reason why I am mentioning this here is that I feel like this suggests a surprisingly easy fix for this long standing issue.

Whenever the user applies an operation that affects multiple rows (such as a reconciliation judgment), we could just preserve those new pagination settings, re-fetch the contents of that page, rerender, and done.

This approach did not make sense before, because preserving the page number is not meaningful: if your action affects earlier rows which are now excluded by a facet, then your view of the grid will jump forward, skipping rows (which is a problem if you are in the process of scanning the grid from start to end, for instance when reviewing reconciliation results). But if we are preserving the row number of the first row to start the page at, this problem disappears. There might still be a little move (for instance because the first row of the page is now filtered away by a facet), but we have the guarantee that no row will be skipped.

Given that we still do not have infinite scrolling, there is also the question of whether the scrolling in the current page should be preserved. Given that the content of the page can still change in pretty arbitrary ways (existing rows becoming filtered out by facets, new rows coming in) I would still automatically scroll back to the top of the page.

Given that it's so simple to implement, I might just do that so that people can try it out - it's probably a lot easier!

@wetneb
Copy link
Member

wetneb commented Nov 4, 2022

This would also solve a related issue, #570.

I realized it's not quite as simple to implement as I thought because if we still want to provide a button to go to the previous page (which we surely do), we need to:

  • either remember on which row that page started - which implies keeping a stack of all previous page starts if the user clicks "Previous" multiple times. And still, that does not give a perfect result because changes in the grid and faceting could mean that the pages get offset.

  • or change the backend so that it supports a different kind of page request, supplying:

    • the engine config
    • the first row number to follow the page to fetch
    • the size of the page

    and still support the other sorts of requests (to go to the next page). With that solution I think we can get exactly the natural, expected behaviour. I think it is doable.

@wetneb wetneb self-assigned this Nov 8, 2022
wetneb added a commit that referenced this issue Dec 5, 2022
…#5411)

* Adapt the GridState interface to separate sorting and pagination.

This adds `getRowsBefore` methods, counterpart to the existing pagination
methods, so that we can do efficient and correct pagination, solving #33 and #570

* Revert "(I #2638) Feature to Goto a page directly (#2639)"

Go back to simply displaying a range of row numbers, because
that will enable a solution for #33, #570 and #572.
This reverts commit d7aaac2.

* Do not move back to the first page when applying an action which refreshes the grid.

Instead, the current page is refreshed.
This closes #33, closes #570, closes #572.

* Specify which actions preserve row and record ids

This lets us preserve pagination only when it is guaranteed to show the same
part of the data after the operation.

* Make paging user-editable like before

* Adapt cypress tests
@wetneb
Copy link
Member

wetneb commented Dec 5, 2022

Closing this issue per #5411 (which is not automatic because it was merged in 4.0, not in master).

@wetneb wetneb closed this as completed Dec 5, 2022
@wetneb wetneb added this to the 4.0 milestone Jun 29, 2023
wetneb added a commit that referenced this issue Oct 10, 2023
…#5411)

* Adapt the GridState interface to separate sorting and pagination.

This adds `getRowsBefore` methods, counterpart to the existing pagination
methods, so that we can do efficient and correct pagination, solving #33 and #570

* Revert "(I #2638) Feature to Goto a page directly (#2639)"

Go back to simply displaying a range of row numbers, because
that will enable a solution for #33, #570 and #572.
This reverts commit d7aaac2.

* Do not move back to the first page when applying an action which refreshes the grid.

Instead, the current page is refreshed.
This closes #33, closes #570, closes #572.

* Specify which actions preserve row and record ids

This lets us preserve pagination only when it is guaranteed to show the same
part of the data after the operation.

* Make paging user-editable like before

* Adapt cypress tests
wetneb added a commit that referenced this issue Nov 12, 2023
…#5411)

* Adapt the GridState interface to separate sorting and pagination.

This adds `getRowsBefore` methods, counterpart to the existing pagination
methods, so that we can do efficient and correct pagination, solving #33 and #570

* Revert "(I #2638) Feature to Goto a page directly (#2639)"

Go back to simply displaying a range of row numbers, because
that will enable a solution for #33, #570 and #572.
This reverts commit d7aaac2.

* Do not move back to the first page when applying an action which refreshes the grid.

Instead, the current page is refreshed.
This closes #33, closes #570, closes #572.

* Specify which actions preserve row and record ids

This lets us preserve pagination only when it is guaranteed to show the same
part of the data after the operation.

* Make paging user-editable like before

* Adapt cypress tests
wetneb added a commit that referenced this issue Jul 29, 2024
* Preserve pagination after actions which trigger an update of the grid (#5411)

* Adapt the GridState interface to separate sorting and pagination.

This adds `getRowsBefore` methods, counterpart to the existing pagination
methods, so that we can do efficient and correct pagination, solving #33 and #570

* Revert "(I #2638) Feature to Goto a page directly (#2639)"

Go back to simply displaying a range of row numbers, because
that will enable a solution for #33, #570 and #572.
This reverts commit d7aaac2.

* Do not move back to the first page when applying an action which refreshes the grid.

Instead, the current page is refreshed.
This closes #33, closes #570, closes #572.

* Specify which actions preserve row and record ids

This lets us preserve pagination only when it is guaranteed to show the same
part of the data after the operation.

* Make paging user-editable like before

* Adapt cypress tests

* Preserve pagination also when discarding recon matches

* Small changes from review feedback

Co-authored-by: Tom Morris <tfmorris@gmail.com>
sunilnatraj pushed a commit to sunilnatraj/OpenRefine that referenced this issue Aug 24, 2024
…fine#6546)

* Preserve pagination after actions which trigger an update of the grid (OpenRefine#5411)

* Adapt the GridState interface to separate sorting and pagination.

This adds `getRowsBefore` methods, counterpart to the existing pagination
methods, so that we can do efficient and correct pagination, solving OpenRefine#33 and OpenRefine#570

* Revert "(I OpenRefine#2638) Feature to Goto a page directly (OpenRefine#2639)"

Go back to simply displaying a range of row numbers, because
that will enable a solution for OpenRefine#33, OpenRefine#570 and OpenRefine#572.
This reverts commit d7aaac2.

* Do not move back to the first page when applying an action which refreshes the grid.

Instead, the current page is refreshed.
This closes OpenRefine#33, closes OpenRefine#570, closes OpenRefine#572.

* Specify which actions preserve row and record ids

This lets us preserve pagination only when it is guaranteed to show the same
part of the data after the operation.

* Make paging user-editable like before

* Adapt cypress tests

* Preserve pagination also when discarding recon matches

* Small changes from review feedback

Co-authored-by: Tom Morris <tfmorris@gmail.com>
@tfmorris tfmorris modified the milestones: 4.0, 3.9 Oct 18, 2024
@magdmartin
Copy link
Member

@tfmorris @wetneb should we re-open to remind us that this needs to be addressed in master ?

@wetneb
Copy link
Member

wetneb commented Oct 23, 2024

It should be addressed by #6546 which was merged in master. Are you still able to reproduce this issue on the master branch? If so, it would be worth reopening this indeed.

@magdmartin
Copy link
Member

It came up as part of the user request in the survey. I wanted to link to the correct issue and provide an update on its status

@thadguidry
Copy link
Member

thadguidry commented Oct 24, 2024

@wetneb I'm trying to test this with master because I do think this was still happening, but not getting far today because I keep getting these console errors when trying to manually "Search for match" (typing in a wood species name, suggest finds it, I click on the QID element, then the dialog goes away, but no match is performed and I get this error):

404 (Not Found)
GET > https://wikidata.reconci.link/search?filter=(all%20mid:Q27684975)&output=(notable:/client/summary%20(description%20citation%20provenance)%20type)&key=null

image

@wetneb
Copy link
Member

wetneb commented Oct 24, 2024

@thadguidry I see you have some operations undone, so you might be affected by #6746. Could you try again with the fix from #6915 applied?

@thadguidry
Copy link
Member

Checked out master, then git pull https://github.com/wetneb/OpenRefine.git 6746-fix-dialog-closing

Still the same 404 error as above in Edge browser console. It throws that 404 error in the console RIGHT WHEN I hover over any suggested QID in the drop down. If I hover over another QID in suggest dropdown that I want to manually apply, I get the repeated error in the console.
image

@wetneb
Copy link
Member

wetneb commented Oct 24, 2024

@thadguidry could you open a separate issue about that then?

@thadguidry
Copy link
Member

thadguidry commented Oct 24, 2024

@wetneb Just tested again with a new project with 7 pages. It works fine, no matter if I click on single or double checkmark, or hover on a suggested match and click on button "Match this Cell", or choose "Search for match" and manually choose a suggested QID from dropdown. The reconciliation stays on the grid page that I am on, and I can navigate to a first or last page, or any page, manually apply a few matches, and it stays on the page I'm on.

The above works no matter if Judgement Facet has a selection on "none" or no selection filtering.

I'd say this is fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grid pagination / scrolling About scrolling through the rows via pagination or potentially seamless scrolling imported from old code repo Issue imported from Google Code in 2010 Module: Frontend These issues involve working on HTML, CSS, and JavaScript code that affects the user interface. Priority: Medium Represents important issues that need to be addressed but are not urgent reconciliation Related to the reconciliation operations and other features Theme: UX/Usability Focuses on issues related to improving the overall user experience and interaction flow. Type: Bug Issues related to software defects or unexpected behavior, which require resolution. Type: Feature Request Identifies requests for new features or enhancements. These involve proposing new improvements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants