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

[Sort] List of records not refreshed when removing sort field #831

Closed
dcfidalgo opened this issue Dec 20, 2021 · 5 comments · Fixed by #1116
Closed

[Sort] List of records not refreshed when removing sort field #831

dcfidalgo opened this issue Dec 20, 2021 · 5 comments · Fixed by #1116
Assignees
Labels
type: bug Indicates an unexpected problem or unintended behavior
Projects

Comments

@dcfidalgo
Copy link
Contributor

To reproduce:

  1. Sort records by a field in the Sort "filter".
  2. Remove the filter by pressing the "x".
  3. You have to press the "Sort" button again to remove the sorting and update the view. For the other filters, pressing the "x" automatically refreshes the view.
    bug_remove_sort
    )
@dcfidalgo dcfidalgo added type: bug Indicates an unexpected problem or unintended behavior app labels Dec 20, 2021
@leiyre
Copy link
Member

leiyre commented Dec 20, 2021

At the moment this filter is different from the rest, having to click "sort" to confirm the action. @Amelie-V, do you think we can remove the filter directly and maybe add a "remove all" like in the rest?

@Amelie-V
Copy link
Member

El cambio mayor deseable es que no haya que clicar un button de validación (Sort) y que el reorden se haga en tiempo real.
Si se quiere esperar la revision global de los filtros que esta pendiente, de momento podemos efectivamente permitir cerrar el modal cuando se da a la cruz

@frascuchon
Copy link
Member

The Sort filter has a different behaviour with X button. For filters, the X button clears all selected fields for that filter. In sort component, the X button drops the sort field configuration, and it used for changing sort configuration before apply it (Like changing selected fields for a filter).

To apply change proposed by @Amelie-V I would like to use the X button in a same way like filters do: clear totally the sort configuration and then close modal. If we decide to close the modal every time we click the X button, changing configuration in a sort with multiple fields could be a bit annoying

Screenshot 2021-12-22 at 12 45 18

@frascuchon frascuchon added type: enhancement Indicates new feature requests and removed type: bug Indicates an unexpected problem or unintended behavior labels Dec 22, 2021
@Amelie-V
Copy link
Member

Would it be possible/relevant to add this logic? @frascuchon @dcfidalgo @dvsrepo @leiyre

  • Add a "remove all sort" when 2 or more sort actived . On click on it, close the modal directly.
  • When there is only one sort actived, click on the X button close the modal too.

@frascuchon
Copy link
Member

Would it be possible/relevant to add this logic? @frascuchon @dcfidalgo @dvsrepo @leiyre

  • Add a "remove all sort" when 2 or more sort actived . On click on it, close the modal directly.
  • When there is only one sort actived, click on the X button close the modal too.

I think the same button should resolve the same action for all cases (clear the Sort configuration) like filter component does. Could you provide a sketch design proposal @Amelie-V ?

Anyway, i think It should be better invest the effort to normalize the filter section

@frascuchon frascuchon added the type: bug Indicates an unexpected problem or unintended behavior label Feb 5, 2022
@frascuchon frascuchon added this to Backlog in Release via automation Feb 5, 2022
@frascuchon frascuchon removed the type: enhancement Indicates new feature requests label Feb 5, 2022
@frascuchon frascuchon moved this from Backlog to Planified in Release Feb 5, 2022
leiyre added a commit that referenced this issue Feb 9, 2022
fix #831
This PR allows to remove sort field from cross button when only one is applied
@leiyre leiyre moved this from Planified to In progress in Release Feb 9, 2022
Release automation moved this from In progress to Done Feb 9, 2022
frascuchon pushed a commit that referenced this issue Feb 9, 2022
This PR allows to remove sort field from cross button when only one is applied

Closes #831

* SortList test

* SorlList snap
@frascuchon frascuchon moved this from Done to Ready to DEV QA in Release Feb 9, 2022
@frascuchon frascuchon moved this from Ready to DEV QA to Approved DEV QA in Release Feb 10, 2022
@frascuchon frascuchon moved this from Approved DEV QA to Ready to Release QA in Release Feb 10, 2022
frascuchon pushed a commit that referenced this issue Feb 10, 2022
This PR allows to remove sort field from cross button when only one is applied

Closes #831

* SortList test

* SorlList snap

(cherry picked from commit f446974)
dvsrepo added a commit that referenced this issue Feb 10, 2022
* 'master' of https://github.com/recognai/rubrix: (26 commits)
  feat(#1061): unify records results title (#1111)
  refactor(#945): using new search service (#1117)
  fix(#1121):  Adjust search bar width (#1124)
  fix(#945): validate label for single label text classification dataset (#1123)
  docs: fix skweak images (#1120)
  fix(#831): Remove sort field when only one is applied (#1116)
  refactor(#945): add current search aggregations as metrics (#1115)
  chore(#982): extends search area (#1112)
  chore(#1054): long records margin adjustment #1114
  feat(#1063): Token Classifier fine tuning content selection (#1084)
  refactor(#1102): remove "Update Summary" button rules summary (#1110)
  refactor(#945): revert index config for text2text (#1108)
  fix: convert pd.NaT to None for event_timestamp (#1105)
  fix(#1094): return empty list for no predicted_as (#1107)
  docs(#1089): remove pip install elasticsearch from docs (#1104)
  fix(#1054): reduce collapsable area. Optimize for annotation (#1106)
  fix(#945): include default aggregations for text2text (#1097)
  refactor(#1044): include last updated field for sort (#1093)
  fix(#1094): remove computed record fields returned in API results (#1095)
  feat(#1051): keep predictions labels when annotating (#1077)
  ...
@leiyre leiyre moved this from Ready to Release QA to Approved Release QA in Release Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Indicates an unexpected problem or unintended behavior
Projects
No open projects
Release
Approved Release QA
Development

Successfully merging a pull request may close this issue.

4 participants