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

Re-trigger hash routing #769

Closed
psinger opened this issue Apr 28, 2021 · 13 comments
Closed

Re-trigger hash routing #769

psinger opened this issue Apr 28, 2021 · 13 comments
Assignees
Labels
feature Feature request ui Related to UI
Milestone

Comments

@psinger
Copy link

psinger commented Apr 28, 2021

Currently, whenever a user clicks twice on the same hash routing navigation, nothing happens the second time.

For example, following scenario:

  • A user clicks on navigation button with hash routing #routing1
  • User navigates to http://localhost:10101/#routing1 which is also the current URL
  • User operates on the new page, but taskbar stays the same as no new hash routing is triggered
  • User clicks again on the same navigation button to reset the page / navigate back
  • Hash routing is not triggered, as active URL is still the same

In simple terms, I would like to always trigger the hash routing, even if the same hash item is currently active.

@psinger psinger added the feature Feature request label Apr 28, 2021
@lo5 lo5 added the ui Related to UI label May 2, 2021
@lo5
Copy link
Member

lo5 commented May 2, 2021

Our front-end implementation is based on the browser's hashchange event. If there is no change in the hash, the change event is not triggered, by definition.

wave/ui/src/app.tsx

Lines 118 to 128 in d1980bb

onHashChanged = () => {
const h = window.location.hash
if (h?.length > 1) {
qd.args['#'] = h.substr(1)
}
qd.sync()
},
init = () => {
connect('/_s', onSocket)
window.addEventListener('hashchange', onHashChanged)
},

We could change the front-end implementation to force-trigger the event regardless of whether it has changed, but this could break existing apps which rely on the current behavior and whose hash routing handlers are not idempotent.

In this case, it seems to me like the correct way to implement flow is: when the user "operates on the new page", it should cause hash changes to something other than #routing1, so that when the user goes back to #routing1, it gets triggered.

For example (changes in bold):

  • A user clicks on navigation button with hash routing #routing1
  • User navigates to http://localhost:10101/#routing1 which is also the current URL
  • User operates on the new page, causing the hash to change to #routing1/action1, #routing1/action2, etc.
  • User clicks again on the same navigation button to reset the page / navigate back
  • Hash routing is triggered, since active URL has changed

@mturoci What do you think?

@mturoci
Copy link
Collaborator

mturoci commented May 7, 2021

Changing hash to **#routing1/action1** seems like a hack to me and pollutes URL. One possible alternative UX could be to use a reset button for resetting the page instead of clicking the same nav item again, although clicking it again is a common UX for this as well.

I have recently faced similar problem in h2oai/wave-apps#57. The problem there is that q.args['#'] is only populated when hash changes. The common routing pattern is:

hash = q.args['#']

if hash == 'page1':
    render_page1()
else:
    render_homepage()

If you, however, perform some actions on page1 (hash doesn't change), you will be redirected to homepage- bug. The problem is that q.args['#'] value serves for 2 things:

  1. Indicate change.
  2. Communicate latest hash.

If we could split this into 2 separate variables (current_hash and hash_clicked), it could give the best DX. I am aware this would be a major breaking change though.

Note: I came up with storing the current hash myself into q.client, but the implementation felt clumsy.

It would also resolve this issue since you would get the hash_clicked flag so dev could rerender UI accordingly.

@lo5
Copy link
Member

lo5 commented May 7, 2021

@mturoci Thanks for the detailed explanation. So, the problem is that "q.args['#'] is only set when the hash changes". Is that correct?

If so, the fix can be "always set q.args['#'] to the current hash, regardless of the hashchange event."

This will mirror the state of the browser's hash in q.args, making the if hash == 'page1' / elif / else switches way more usable than they currently are, without a breaking change.

@psinger I understand your problem much better now :)

@mturoci
Copy link
Collaborator

mturoci commented May 7, 2021

@lo5 correct

@psinger
Copy link
Author

psinger commented May 26, 2021

If so, the fix can be "always set q.args['#'] to the current hash, regardless of the hashchange event."

Is this something that is planned to be implemented on Wave side, or something that is doable on app development side?

@vopani
Copy link
Contributor

vopani commented May 26, 2021

I would also prefer always setting q.args['#'], it is useful to provide that and let developer decide how to handle. I think in current scenario, it won't break any app (maybe just some additional / simplifying handling of tabs might be required).

@mturoci
Copy link
Collaborator

mturoci commented May 26, 2021

Is this something that is planned to be implemented on Wave side, or something that is doable on app development side?

@psinger This is something that needs to be implemented on Wave side.

@psinger
Copy link
Author

psinger commented May 26, 2021

@mturoci alright great, hope this will be added

@lo5 lo5 added this to the 2021 milestone Jun 29, 2021
@lo5 lo5 self-assigned this Jun 29, 2021
@lo5 lo5 closed this as completed in bc409b3 Jun 30, 2021
lo5 added a commit that referenced this issue Jun 30, 2021
@psinger
Copy link
Author

psinger commented Jul 1, 2021

@lo5 thanks for implementing I just have two follow-up questions:

I understand that q.args['#'] is now set all the time, but it is still not re-triggered if I click the same navigation button again. So if I am currently at hash page1 and click the according navigation button again, nothing happens. In order to solve the original use case, it would be necessary to re-trigger the navigation button if you click it again.

The second question is, if there is some way to flag whether a navigation button has been actively clicked, otherwise it is hard to escape the generic if sequence of:

if q.args["#"] == "page1":
    await do_something(q)
elif ...

Edit:
using ui.command for navigation buttons

@AakashKumarNain
Copy link

AakashKumarNain commented Jul 2, 2021

So if I am currently at hash page1 and click the according navigation button again, nothing happens

I checked this as well yesterday and I can confirm that re-triggering isn't working as expected

@lo5
Copy link
Member

lo5 commented Jul 2, 2021

The issue title is misleading. The fix does not re-trigger links (see explanation: #769 (comment)). Instead, the fix sets q.args['#'], whether it has changed or not (see #769 (comment)).

Regardless, @psinger @AakashKumarNain, help me understand what's the need for the browser to trigger the app twice when clicking on the same link twice in succession. If that's the behavior you need, then the command should be using plain names (name='foo' instead of name='#foo') so that the commands behave like buttons and not hyperlinks.

Additonally, from v0.17+ if you need to detect changes to the '#' link, for whatever reason, you can do something like this:

is_hash_changed = q.client.hash != q.args['#']
q.client.hash = q.args['#']

@psinger
Copy link
Author

psinger commented Jul 4, 2021

The use case is exactly how I described above. If you perform any other action on a page (non navigation) that leads you to a different part of the app, you might want to go back, and the natural thing to do is to click on the same navigation button again. But the hash never changed, because you did not perform navigation before, and apparently the navigation does not re-trigger in that case.

Regarding your suggestion with detecting changes in hash: this also only works if the hash actually changed, not when the user re-clicked the same navigation button (which currently does not trigger though).

@AakashKumarNain
Copy link

AakashKumarNain commented Jul 4, 2021

To elaborate a bit more, I would give you an example. Let's say you are on a page, say #setup. Any page can have n number of states, for example:

  1. Configure the parameters
  2. Configure the model params
  3. Configure the machine
    ...

At every state, we can provide an option to go back to the last state but if we ask the end user to use the same back button to go from state n to state 1, then it isn't a very user-friendly experience. We can provide a reset button as well but that will be misleading as reset can convey two meanings: reset the current state or go back to the initial setup

Neither it is natural nor it is user friendly IMHO. More natural is to click the same tab so that you come back to the vey first state with that click

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request ui Related to UI
Projects
None yet
Development

No branches or pull requests

5 participants