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

Refactor JDocument so it can be autoloaded #8905

Merged
merged 11 commits into from Jan 15, 2016
Merged

Refactor JDocument so it can be autoloaded #8905

merged 11 commits into from Jan 15, 2016

Conversation

wilsonge
Copy link
Contributor

To test ensure that Joomla itself functions the same (just try a couple of pages). The error page works and any custom formats you might have (e.g. the VCF view for com_contact etc).

There should be no changes before and after the patch, however in the future you can now autoload properly in JDocument. Having things like the renderers now running autoloading is going to be important in the future for introduction of alternate output formats.

{
/**
* @var JDocumentJSON
\ */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a removed line. So all good now :)

@izharaazmi
Copy link
Contributor

👍 for adding support for JDocumentRenderer[doc-type][rendered-type]. How about now providing a standard set of JDocumentJson renderers? Should we?

@photodude
Copy link
Contributor

I have tested this item ✅ successfully on aa7c947

As per test instructions, tested that Joomla itself functions the same


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8905.

@wilsonge
Copy link
Contributor Author

@izharaazmi something like that won't be part of this PR. As long as people are extending the right classes (which we have checks for) then it's down to them to provide an implementation. In the future we could investigate some sort of interface. But it's likely going to be something JDocumentRenderer already supports anyhow

@izharaazmi
Copy link
Contributor

Yeah, I understand that that can't be part of this PR. I was asking for the additional prospects. I think we can provide a full fledged structured rendering similar to that of html renderers, like response code, messages, body, redirects, callbacks, etc or whatever we can figure out when into it.

This can give us a well defined format for Joomla based JSON APIs as well. Same can be done for XML rendering for XML APIs.

I understand that this is not the part of Joomla project scope right now. But I wonder, what if?

Btw, JDocumentJSON right now appears to be just a barebone (sorry! I may be wrong here). It renders the output with Content-Disposition: attachment anyway, which usually is not helpful always.
However, these can be discussed later in case we actually decide to work on this.

@wilsonge
Copy link
Contributor Author

Exactly. There's definitely a lot of work to do with JDocument. I see this as the start of getting a lot of these issues fixed up

@mbabker
Copy link
Contributor

mbabker commented Jan 15, 2016

JDocument and its subclasses aren't a proper request/response structure, nor even responsible for building the response. It is at best a page rendering system; all of the response data is handled as part of the JApplicationWeb API. JDocument does have a couple of calls into the application to set some response data based on the format, but that's the extent of what it's capable of at the moment.

@anibalsanchez
Copy link
Contributor

I have tested this item ✅ successfully on aa7c947

I have been working with this patch, it works OK.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8905.

wilsonge added a commit that referenced this pull request Jan 15, 2016
Refactor JDocument so it can be autoloaded
@wilsonge wilsonge merged commit 06200e9 into joomla:staging Jan 15, 2016
@mbabker mbabker deleted the JDocumentRefactor branch January 15, 2016 20:28
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.

None yet

7 participants