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

[API] onResponse was not obeying return dataType #3653

Closed
wants to merge 1 commit into from

Conversation

bockscar
Copy link

@bockscar bockscar commented Feb 4, 2016

I'm new to Semantic and trying to get brain around a few concepts. Thank you for this awesome work.

I am exploring using Semantic with a legacy backend. The legacy backend returns JSON that requires processing. One key in the returned JSON is 'html' from which, say a tab's HTML contents, would be pulled.

Though my question applies to all API calls, running with tabs is a useful case study. A tab is hardwired to seek and get returned from server plain HTML. My objective would be to leverage as much Semantic tab logic as possible but get it to work with this legacy JSON backend.

This discussion regarding a related topic discusses the use of 'mockResponseAsync' for a related purpose:
#1736

It seems onResponse is the simpler solution. I was glad to find it. But I did not understand why the response text was being transformed into a hash before being sent to the 'onResponse' callback. The first thing I need to do on the other end is reassemble the original response text so I can process it in proprietary fashion. Maybe there's a reason for this transformation?

My suggestion retains the default and offers a flag to skip this wasteful transformation so it needn't happen every server hit if it is not necessary.

Maybe I misunderstood 'mockResponseAsync' and that's where I should be working? If not, maybe the default behavior for 'onResponse' transforming the text might be confusing to others as well.

Thanks for the awesome work. I look forward to getting deeper. Even without this change, it's cool that hooks are in place so I can wire Semantic into a legacy app.

@jlukic
Copy link
Member

jlukic commented Feb 5, 2016

If you could create a text case that reproduces the specific issue with onResponseAsync it'd be easier to talk about.

Most modules that consume API results allow for field mappings in JSON using the fields settings object.
http://semantic-ui.com/modules/search.html#using-different-response-fields

@bockscar
Copy link
Author

bockscar commented Feb 5, 2016

Sorry. Wanted to include some info on the use case if it happened to apply to anyone else. The callbacks actually do exactly what we need (aside from this minor friction).

Heart of the question was understanding why response is passed after being converted to an object. Other places response is passed as a string. That's why this usage stood out and confused me initially...because I was expecting a string:
settings.onResponse.call(context, $.extend(true, {}, response))

@jlukic
Copy link
Member

jlukic commented Feb 28, 2016

It looks like this might be a bug when dataType is not specified to the default JSON. I'll have to take a look.

I don't think we need a separate setting, it should just mimic the expected response type.

@jlukic jlukic added this to the 2.1.9 milestone Feb 28, 2016
@jlukic jlukic changed the title Add ability for 'onResponse' to pass plain response text rather than a hash [API] onResponse was not obeying return dataType Feb 28, 2016
@jlukic jlukic closed this Feb 28, 2016
@jlukic
Copy link
Member

jlukic commented Feb 28, 2016

I've taken a different angle, but this should fix it as long as you are specifying a different dataType than JSON or JSONP

@jlukic jlukic modified the milestones: 2.1.9, 2.2 May 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants