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 property is not removed from URL after dismissing a relation modal with tabs #1140

Open
goldmont opened this issue Jun 3, 2024 · 11 comments

Comments

@goldmont
Copy link

goldmont commented Jun 3, 2024

Winter CMS Build

1.2

PHP Version

8.1

Database engine

MySQL/MariaDB

Plugins installed

No response

Issue description

Hi,

When using tabs inside relation modal, the hash property doesn't get removed from the URL when the modal get dismissed. Thus, clicking on another record in the list make the modal open to the last tab visited.

Thanks.

Steps to replicate

  1. Configure a relation (in my case a Belongs-To-Many).
  2. Configure a relation controller.
  3. Configure a form with some tabs.
  4. Use the relation modal.

Workaround

No response

@AIC-BV
Copy link
Contributor

AIC-BV commented Jun 3, 2024

Can confirm the behaviour, only wondering if it is a bad or a good thing.
I think in my case it is handy that the correct tab is already opened

But I assume you want it to 'reset' to open the correct (first in this case) tab for you

@bennothommo
Copy link
Member

UX-wise, modals should generally be considered a new "user session", in terms of its scope being separate to that of the main form. I feel with that, it would mean that each time the same modal opens, it should still be considered a new session, thus it should probably default back to the first tab and not consider any "shared scope" with the previous usage of the modal - essentially, modals shouldn't save the selected tab in the URL at all.

@LukeTowers
Copy link
Member

I'd have to disagree slightly, it can be very useful to have the modal open to the same tab as it was previously especially when clicking through related records to make a change on a specific tab one after another.

I do agree though that we should move away from storing the selected tab state in the URL, it would be better to tie it more to the context of a given tab container instead.

@bennothommo
Copy link
Member

That makes an assumption that the same tab is available on all records. It might not be, depending on the state of the record being viewed (for example, hidden due to trigger rules or filter fields, etc.)

Also, the normal flow between lists and forms for the main model doesn't recall the last tab being used when going from one record to the next. Your suggestion for storing the context somewhere else might make that work, and that would be fine, but we should make it a consistent experience across both.

@LukeTowers
Copy link
Member

@bennothommo I sometimes implement a "next / previous" button in my form pages, in which case you would keep the tab context between records.

Any ideas for other ways to store the tab context?

@AIC-BV
Copy link
Contributor

AIC-BV commented Jun 7, 2024

I'd have to disagree slightly, it can be very useful to have the modal open to the same tab as it was previously especially when clicking through related records to make a change on a specific tab one after another.

This is exactly the case for us

I do agree though that we should move away from storing the selected tab state in the URL, it would be better to tie it more to the context of a given tab container instead.

And this will be a great improvement. I've had feedback about this. My collegues often increase the id at the end of the URL by one or change it to the order they need, but it is more difficult for them if there's a hash after the ID

@LukeTowers
Copy link
Member

My collegues often increase the id at the end of the URL by one or change it to the order they need

@AIC-BV this might be helpful for you: https://wintertricks.com/tricks/display-next-previous-record-buttons-in-a-form

Screenshot 2024-06-07 at 10 13 52 PM

@AIC-BV
Copy link
Contributor

AIC-BV commented Jun 10, 2024

My collegues often increase the id at the end of the URL by one or change it to the order they need

@AIC-BV this might be helpful for you: https://wintertricks.com/tricks/display-next-previous-record-buttons-in-a-form

Yes I have something like that but they don't really use it

<?php
if (isset($formModel)) {
    $prev = \Aic\Account\Models\Order::where('id', '<', $formModel->id)->max('id');
    $next = \Aic\Account\Models\Order::where('id', '>', $formModel->id)->min('id');
}
?>
...
<?php if ($next): ?>
  <a
      href="<?= $next; ?>"
      class="wn-icon-angle-right btn-icon pull-right"
      style="font-size: 25px; text-decoration:none"
      title="<?= e(trans('aic.account::lang.button.next')) ?>">
  </a>
<?php endif ?>
  
<?php if ($prev): ?>
  <a
      href="<?= $prev; ?>"
      class="wn-icon-angle-left btn-icon pull-right"
      style="font-size: 25px; text-decoration:none"
      title="<?= e(trans('aic.account::lang.button.previous')) ?>">
  </a>
<?php endif ?>

@AIC-BV
Copy link
Contributor

AIC-BV commented Jun 10, 2024

I do agree though that we should move away from storing the selected tab state in the URL, it would be better to tie it more to the context of a given tab container instead.

To continue the discussion, I noticed a 'breaking change' in my (future) case:

  1. I want to create a link from modal A to B (Account/Order). Model B hasMany model A.
  2. And the link has to open the correct tab in Model B. For example:
    $url = Backend::url('aic/account/order/preview/' . $this->order_id . '#primarytab-aicaftersaleslangmenuaftersales');

In this case, keeping the tab state in the URL would make sense, or atleast the keep the functionality behind it alive (automatically clicking the tab).

I totally agree the URL gets dirty the current way though.

@bennothommo
Copy link
Member

bennothommo commented Jun 11, 2024

EDIT: Sorry, @AIC-BV, I realised after posting that you made this exact point :P

I assume the hash was used so that if people copy and paste the URL and provide it to someone else, when they use that URL, it'll open up the applicable tab. If that's a goal of this functionality, then unfortunately there's no way around using hashes.

If we're not so concerned with that, and only want the tab to be remembered for the current user, then we can use something like Local storage or Session storage, combined with a form ID, to store the state of the tabs for each form. Either would take the state out of the URL that way.

@LukeTowers
Copy link
Member

I believe "tab deeplinking" is the original reason why the URL hashes were added, although I'd have to go back through the commits to verify. Maybe it could be made configurable?

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

No branches or pull requests

4 participants