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

Hash history #3923

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Hash history #3923

wants to merge 4 commits into from

Conversation

ugurbahadir
Copy link

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

Implements node-red/designs#79 to support URI fragments getting updated by using the browser's history API.
The browser's forward and back keys allow navigation to improve user experience.
In addition, it is possible to determine the interface elements to be tracked.

hash.history.mp4

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 16, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@knolleary
Copy link
Member

Hi @ugurbahadir

thanks for this PR. As per our contribution guidelines, we prefer to discuss new contributions before they arrive so we can make sure the proposed changes fit with what we have in mind.

In this instance, you've added a lot of stuff that wasn't in the original design - particularly providing the user with lots of options over what gets tracked.

The implementation of the design that we already have in dev was done to get the balance right over convenience for the user and not have the uri constantly changing as they click around.

I can see some merit in hooking into the browser history api - I think that is something I had mentioned would be a nice to have - but its hard to accept this PR with all of the other parts you've added.

@ugurbahadir
Copy link
Author

Hi thanks for your feedback, you are right the implementation of the design was done but at the end of the design there is an open point.

The outstanding question is how the fragment then gets updated whilst using the editor. I'm not sure updating it to reflect the current selection is a good idea or not

I just wanted to produce an experimental solution for this point without bothering users. tracking every selection may bother but in the final product, instead of following all the movements, only the tab changes can be used as the default setting. More tracking can be provided according to user preference.
If you can give feedback on the changes I've made, I can work on it. or you can just close this PR without merging. have a good weekend

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.

None yet

2 participants