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

Back Button is not working proper in the camera configuration page #9233

Closed
nihal467 opened this issue Nov 28, 2024 · 5 comments · Fixed by #9246
Closed

Back Button is not working proper in the camera configuration page #9233

nihal467 opened this issue Nov 28, 2024 · 5 comments · Fixed by #9246
Assignees

Comments

@nihal467
Copy link
Member

Describe the bug

The back button on the Camera Configuration page does not take the user to the previously accessed page.

To Reproduce
Steps to reproduce the behavior:

  1. Go to the Assets section.
  2. Click on any camera asset.
  3. Click on the Configuration button, then click on the back button in the breadcrumbs.
  4. See error

Expected behavior

The back button in the breadcrumbs should navigate the user to the previously accessed page.

Screenshots

Image

@SwanandBhuskute
Copy link
Contributor

I would like to work on this @nihal467

@github-project-automation github-project-automation bot moved this to Triage in Care Nov 28, 2024
@rithviknishad rithviknishad moved this from Triage to Up Next in Care Nov 28, 2024
@SwanandBhuskute
Copy link
Contributor

@nihal467 @rithviknishad
I think this is the error

If the location of the asset has bed in them
then an additional query of ?bed=..... is added in the current url (after clicking on Configure, what we see that page) even if no bed if link

but if the asset facility doesn't have any bed in them, then the url is normal like /facility/sdu-sf-dsf-fdf/assets/dsf-sdfds-0dsf0/configure

so
if url - facility/abc-xyz-pqr/assets/abc-112-sdf-prd/configure , "Back" works fine
if url - facility/abc-xyz-pqr/assets/abc-112-sdf-prd/configure?bed=tru-23-dsf-3e4 , "Back" not working

@github-actions github-actions bot added needs-triage question Further information is requested labels Nov 28, 2024
@rithviknishad
Copy link
Member

rithviknishad commented Nov 28, 2024

Yes, we are using the query param to store the state for currently selected bed to be configured. We could skip the setting of query param when the asset bed list request completes initially, instead just use the first bed as the fallback if no query param is set.

@SwanandBhuskute
Copy link
Contributor

SwanandBhuskute commented Nov 28, 2024

This is the error @rithviknishad @nihal467

The history is like this
[
"/facility/0c95c7f0-e1d2-4aff-83fa-933cef60d3a8/assets/5b089eb8-0b23-4d47-ae86-337a3902672e/configure?bed=f3323a4f-dae4-4c8a-a3fa-ac980d4adf34",
"/facility/0c95c7f0-e1d2-4aff-83fa-933cef60d3a8/assets/5b089eb8-0b23-4d47-ae86-337a3902672e/configure",
"/facility/0c95c7f0-e1d2-4aff-83fa-933cef60d3a8/assets/5b089eb8-0b23-4d47-ae86-337a3902672e",
"/assets",
"/facility"
]

so , when we go on /configure
we immediately fetch the first linked bed or unlinked bed (if not) and attach it in the url and thus the history gets updated

then when I click back
I do go to /configure again, but as we are by default fetching first bed, again the url gets updated and so is history

so you can see
we are stuck in this infinite jumps of
history[0] - /configure?bed=
and
history[1] - /configure

so i think we need to handle this useEffect hook

Image

maybe we can just not change the url on initial render , keep it /configure?
it makes sense too

@github-actions github-actions bot added needs-triage question Further information is requested labels Nov 28, 2024
@rithviknishad
Copy link
Member

Let's remove that useEffect hook (line 96 to 100) altogether. And modify the selectedAssetBed to first check from query param and if query param not present, fallback to using the firstBedId without needing to set the query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants