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

OnDemandGrid does not provide error handlers to store.query() #241

Closed
neekfenwick opened this issue Jul 27, 2012 · 1 comment
Closed

OnDemandGrid does not provide error handlers to store.query() #241

neekfenwick opened this issue Jul 27, 2012 · 1 comment

Comments

@neekfenwick
Copy link

While writing my own store that returns a Deferred, I realised dgrid didn't use the def.reject() handling I was coding into it. I've put simple error handling into my OnDemandList and it seems to function. I'm far from expert with Deferred or dgrid so I'm sure at least some of this approach is wrong.

See attached patch.

  • Add an errorMessage property to OnDemandGrid.
  • Provide a third argument to the two calls to query() to handle rejected Deferred in case of error.
  • Insert a node into the dgrid to indicate to the user that an error occurred.
  • Put a new class 'error-message' on that message div to allow custom styling (by the way, no custom styling on the current 'loading' message?)

Notes:

  • Very lazily, I'm creating a new node below the 'loading...' message node, so you end up with both the 'loading...' message and the error message. It's my first time with put-selector and I didn't spend enough time to figure out how to cleanly remove the loading message before inserting the error one.
  • I've not looked beyond OnDemandGrid to implement this, perhaps it needs a better or more integrated fix impacting other dijits.
  • The test case that uses OnDemandGrid with a store that uses Deferred (OnDemand_promises.html?) probably needs updating to reject a Deferred and exercise these new code paths.
@ghost
Copy link

ghost commented Dec 19, 2012

Thanks for the report, and sorry for taking so long to get back around to this. I know I did discuss briefly with you on IRC a while ago, but I just came back around to this because #351 actually resolves at least part of the issue - specifically the issue regarding not having appropriate error callbacks hooked up, which had the propensity to leave loading nodes hanging around.

Error messages were left as an exercise to the interested developer; the thing about leaving error messages hanging around especially in the case of OnDemandList is that it's not entirely clear when they should be removed, and having to clean them up in addition to the usual re-querying of rows / adjusting of preload nodes would add complexity for questionable gain.

All code which automates store requests should be going through _StoreMixin#_trackError, which emits a dgrid-error event if something goes wrong. Developers can react to this event as they please to customize the behavior of their UI.

Regarding styling of the loading message, again this was left as an exercise to developers, since usually people tend to disagree on what they want their loading indicators to look like. :) You can add styles to the .dgrid-loading class to customize this as you please.

I'll leave a comment on #242 as well, but I'll be closing these issues now. Thanks for the input though, and feel free to open further issues if you find any (or hit me up on IRC).

This issue was closed.
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

No branches or pull requests

1 participant