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

Add ability to navaigate to specific tab in target view. #701

Merged
merged 3 commits into from
Nov 27, 2023

Conversation

davner
Copy link
Contributor

@davner davner commented Oct 18, 2023

Resolves Issue #697.

  1. Introduces URL parameter support for directly navigating to specific tabs in the target detail view (e.g., /targets/1/?tab=observations). Initial implementation is limited to two sections that our users found most impactful but could be applied anywhere the directs to the target detail view. This was written with vanilla javascript, removing the concern of Jquery conflicts.

  2. Augments observation_list.html to display observation IDs, improving user experience to trace data to what ID in the overview.

@phycodurus phycodurus linked an issue Oct 24, 2023 that may be closed by this pull request
Copy link
Contributor

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like these features, and I think this new functionality is fantastic, but I don't see how this directly addresses Issue #697.
The active tab information is still lost on refresh, correct?
Does this functionality override that issue, or should the issue persist once this PR is merged?

@davner
Copy link
Contributor Author

davner commented Nov 18, 2023

@jchate6 , you've raised a valid point. The problem still persists, as this PR provides enhancements, but doesn't address the original issue. I've given this some thought and have something. I do want to avoid local storage because I don't want the tab to persist upon navigating away.

  1. Event Listener on Tabs: I will add an event listener to the tabs.
  2. URL Update on Tab Click: When a tab is clicked, the URL will be updated to reflect the active tab. This ensures the user stays on the same tab after certain actions.
  3. Page Refresh Behavior: Refreshing the page will take the user back to the last active tab, as indicated by the URL.
  4. No Local Storage Dependency: We won't use local storage, so the last opened tab won't persist if the user navigates away. This keeps the experience clean and straightforward.
  5. Specific Tab Redirects: The solution still allows for redirecting to a specific tab when needed.

Our URLs would look like

Thoughts? I will push this to this PR first thing next week along with your comment being addressed.

@jchate6
Copy link
Contributor

jchate6 commented Nov 18, 2023

Avoiding local storage makes a lot of sense.
I agree that an event listener on the tab is the way to go.
Thanks!

@davner
Copy link
Contributor Author

davner commented Nov 21, 2023

@jchate6 Comments have been addressed and tab is remembered on reload.

@jchate6
Copy link
Contributor

jchate6 commented Nov 21, 2023

Thanks @davner! I probably won't get to this until Monday.

Copy link
Contributor

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great.
Thanks for this @davner.
I think we'll probably want to take some time in the future changing redirects and maybe adding a default option to settings.py so users can choose their default landing page for their TOM. Very exciting.

@jchate6 jchate6 merged commit f0355ce into TOMToolkit:dev Nov 27, 2023
12 checks passed
@davner davner deleted the tab-pane-url branch November 27, 2023 23:03
@davner
Copy link
Contributor Author

davner commented Nov 28, 2023

@jchate6 Not a problem. I'm trying to push back all the changes I make for our project that would be beneficial to all users. Glad you like this one!

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.

Tab pane not set in local storage for /targets/1
2 participants