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

Tags loading #792

Merged
merged 35 commits into from
Dec 11, 2024
Merged

Tags loading #792

merged 35 commits into from
Dec 11, 2024

Conversation

xIrusux
Copy link
Contributor

@xIrusux xIrusux commented Dec 3, 2024

Changes in this pull request

Resolves #725 and #730 and #724

Additional info

  • Tags have been allocated so we are only fireing the required POST's / DELETE's when adding / removing tags
  • The Tags cache gets invalidated when the Tabs component gets re-rendered (i.e. when another user makes a change to the tags, we have to close the tab and reopen it to see the change )
  • There is an error snackbar / message at the bottom of the screen if assigning / un assigning a tag does not work
  • In case of error we also undo the optimistic loading
  • The Tag checkbox enters a loading state during POST/DELETE and further actions are not allowed during that time
  • There is no more cross refreshing when other assets with the tags tab open are opened as per 730

@xIrusux xIrusux requested a review from vin0401 December 3, 2024 08:58
@xIrusux xIrusux requested a review from markus-moser December 4, 2024 14:17
@xIrusux
Copy link
Contributor Author

xIrusux commented Dec 4, 2024

@markus-moser
Since we last spoke I have simplified the rtk tag set up and understood a few things about it I previously did not :).
I also found that the Problem with the indicator for clicking the trash bin is a bit more complex than I thought. This is something I am solving as part of a separate pull I am currently working on.

This changed notably:

  • The POST/ DELETE requests when clicking the Tag checkboxes now have a GET response at the end. This is only one response and ensures the optimistic update gets verified and therefore logically makes sense.
  • I also removed the getElementDetailTag from the Tags and Notes and Event reloading invalidation Tags. This is as we do not seem to require a reload of the Asset when adding or removing Notes.

Please let me know if you have any questions or there is something we should discuss :).
Thanks

@markus-moser
Copy link
Contributor

@xIrusux Thanks for the update, looks much better. Unfortunately I still sometimes have a strange behaviour with the checkbox states. Not sure if this is somehow connected with the GET request which refreshes it at the end. Please take a look at the video (I throttled down the network speed a bit for the video):

Bildschirmaufnahme.2024-12-04.um.16.13.03.mov

@xIrusux
Copy link
Contributor Author

xIrusux commented Dec 5, 2024

Taking another look at this.
Thanks for testing!

@xIrusux
Copy link
Contributor Author

xIrusux commented Dec 11, 2024

@markus-moser

  • tag deletion does now work and not fall over itself
  • the results are ordered alphabetically and one can choose the desc / asc in column header. (There are slight differences in how the Tag tree on the left and the grid on the right are handling upper cases as children item paths i.e. ("Color/Red" vs "Color/red") In the Tag tree this is always put at the first place. While in the Grid the upper case is not always treated the same way.
Screen.Recording.2024-12-11.at.12.51.23.mov
  • In a separate issue I am exploring a separate ordering function and also a solution to the "Schedules" Tab not updating.

  • I tested the other Tabs and did not find the same refresh issue except for "Schedules"

  • When the assign or unassign fails - the optimistic loading reverts back to display the previous state

Copy link

sonarcloud bot commented Dec 11, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@xIrusux xIrusux merged commit 1747680 into 1.x Dec 11, 2024
1 check passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tags] Loading indicator
3 participants