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

[dev.icinga.com #13115] actiontable should not clear active row in case there is no newer one #2615

Closed
icinga-migration opened this issue Nov 9, 2016 · 8 comments
Labels
area/javascript Affects the javascript framework bug Something isn't working
Milestone

Comments

@icinga-migration
Copy link

This issue has been migrated from Redmine: https://dev.icinga.com/issues/13115

Created by tgelf on 2016-11-09 16:41:44 +00:00

Assignee: (none)
Status: Resolved (closed on 2016-12-09 12:50:04 +00:00)
Target Version: 2.4.0
Last Update: 2016-12-09 12:50:04 +00:00 (in Redmine)


Currently it removes all class="active" and then tries to figure out whether there is a new candidate that should be elected active. Blind guess: this is probably only a quick hack to solve issues on the dashboard, as it formerly didn't work like this (long time ago). We should IMO get this corrected.

This is confusing when:

  • you work with modules providing an overview table and lot of activity, different controllers, many things that could be "accomplished" on the "right side". You select something, see the object, want to get some work done on it - and loose the "focus", the row selection
  • a module decides to set a specific row active on it's own at server side for a specific reason. Imagine a "go back" action (not in the browser, just a link) that will make the details disappear and show the former table again. I want to highlight the row that has been clicked before, and setting class="active" at server side would be the most intuitive solution to this. Everything else leads to confusion and duplicated CSS.

Hint: when you try to work around this, issue #13113 makes the whole thing even worse.

Changesets

2016-11-09 19:16:00 +00:00 by tgelf d2710cf

actiontable: gracefully clear and improve...

...state preservation

refs #13115

2016-11-10 00:40:42 +00:00 by tgelf 2d58c85

actiontable: gracefully clear and improve...

...state preservation

refs #13115

2016-11-16 09:38:44 +00:00 by tgelf 0f58776

actiontable: gracefully clear and improve...

...state preservation

refs #13115

2016-12-09 12:49:27 +00:00 by elippmann aa559c4

Merge branch 'bugfix/actiontable-row-handling-13115'

fixes #13115

Relations:

@icinga-migration
Copy link
Author

Updated by elippmann on 2016-11-09 16:48:22 +00:00

Hi Tom,

When does this happen?

Best regards,
Eric

@icinga-migration
Copy link
Author

Updated by elippmann on 2016-11-09 17:08:55 +00:00

  • Category set to JavaScript
  • Status changed from New to Feedback
  • Assigned to set to tgelf
  • Target Version deleted 2.4.0

Is this something we've introduced after 2.3.4?

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-11-09 17:13:47 +00:00

Don't think so, I got angry about this many times before :D But in "normal live" it is just confusing. Today however I wanted to create a new feature and haven't been able to do so because of this issue, so I thought time was right to finally tackle this.

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-11-09 18:11:04 +00:00

  • Status changed from Feedback to New
  • Assigned to deleted tgelf

Some more details

refresh() basically does clear(); and selectUrl(); - I'd prefer to see something like if (newRow = findNewRow()) { selectRow(newRow); } - with clear(); being called in selectRow. Please note that this might involve some logic dealing with other related tables, read "dashboard". That one currently works find as of clear & select, it wouldn't anymore when fixing this.

Now lets move on with the problem. In selectUrl() we finish here:

// rows sometimes need to be displayed as active when related actions
// like command actions are being opened. Do not do this for col2, as it
// would always select the opened URL itself.
if (this.col !== 'col2') {
    this.rows().filter('[href$="' + icinga.utils.parseUrl(url).query + '"]').addClass('active');
}

This is a workaround for my problem, but it works out for very simple tab constructs on the right side, with all of them having the same parameters. This doesn't work out for the Director, as it often involves different controllers on the right side while keeping the very same table on the left. You might navigate to a host, show it's services, click a service - the row gets deselected, while your are still in the same context. Or go to deployments, choose one, show it's config. Which one to click next? You have no idea which row you are currently working with.

Fixing this issue would also allow me to preselect a specific row. Click a specific config file belonging to a deployed config. It will be rendered in the same container and provide a back link. What I'd like to do is modifying that back-link in a way it would pre-select the row you formerly clicked, making it easier to figure out where you started your investigation. So, basically that's it where this issue started - because I'm currently unable to implement this.

But regardless of this feature, the current behavior of toggling the selected row on-off-on-off when navigating through the tabs in the right container is confusing and should be fixed.

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-11-09 19:17:53 +00:00

I pushed feature/actiontable-row-handling-13115 - tries to address all mentioned issues.

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-11-09 19:39:38 +00:00

  • Relates set to 13119

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-11-16 09:35:07 +00:00

  • Target Version set to 2.4.0

@icinga-migration
Copy link
Author

Updated by elippmann on 2016-12-09 12:50:04 +00:00

  • Status changed from New to Resolved
  • Done % changed from 0 to 100

Applied in changeset aa559c4.

@icinga-migration icinga-migration added bug Something isn't working area/javascript Affects the javascript framework labels Jan 17, 2017
@icinga-migration icinga-migration added this to the 2.4.0 milestone Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/javascript Affects the javascript framework bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant