The admin pages and functions should be moved into separate (quicktabs.admin.inc) file. This way less code will be loaded into the memory.

Comments

katbailey’s picture

Hi Pasqualle,
thanks for all your suggestions - I hope to start incorporating them very soon. Right now I'm focusing on the 6.x-2.x branch which is very experimental - but I have been doing lots of refactoring of code as well and agree that more is needed.

regards,
Katherine

pasqualle’s picture

Title: separate the admin pages » separate the admin pages +other fixes and improvements
Priority: Normal » Critical
Status: Fixed » Needs review

OK, I just wanted to fix this issue but ended up rewriting the whole module. Please review.

changes:
01. fixed: code style
02. fixed: php doc
03. new: quicktab.admin.inc file
04. fixed: PHP notices
05. new: better functions to list enabled views and view displays
06. fixed: quicktabs_form parameters (the first paremeter in _forms functions should be $form_state)
07. new: created %quicktabs wildcard in the menu
08. new: different style for different QTabs (still not possible to show different QTabs on the same page, that would require a change in the styles)
09. fixed: invalid XHTML on admin/settings/quicktabs
10. new: create QTabs dynamically with quicktabs_render()
11. new: the quictabs preview changed from hardcoded to quicktabs_render() function
12. new: less tab data saved into db
13. new: node can be a tabpage
14: fix: removed code duplication (there is absolutely no difference between ajax and non ajax QTabs admin interface, just a single flag)
15: new: excel style included
16: fix: unable to delete tabpage

and other small changes

I can provide a patch if necessary, but as I said I changed the whole module, so it is impossible to visually review the patch..

EDIT: download link removed (new patch here: #339481: various improvements and fixes )

katbailey’s picture

Title: separate the admin pages » separate the admin pages +other fixes and improvements
Version: 6.x-1.x-dev » 6.x-2.x-dev
Priority: Normal » Critical
Status: Active » Needs review

Hi Pasqualle,
hmm, I spent a good few hours last overhauling the 6.x-2.x version of the module, including a lot of the above (moving admin functions to separate admin include file, refactoring the QT creation/edit form to eliminate repeated code), as well as using the refactored ahah callback stuff from the 1.x branch and adding support for ajax-paged views in ajax-driven quick tabs. So this seems a pity to have had two of us working on the same thing. However, I'm delighted to have somebody other than myself spending time on this as I'm finding it hard to find time to make all the changes that are needed. I guess the next step is for me to go through your changes, see where they overlap with mine and somehow meld the two together in a way that takes the best of both versions.

thanks,
Katherine

katbailey’s picture

Title: separate the admin pages +other fixes and improvements » separate the admin pages
Status: Needs review » Fixed

On second thoughts, it's not going to work that way. I'm changing the title back because the module has changed quite a bit from what you re-worked. I have committed my changes because they were mainly focused on enhancing the functionality (getting ajax-paged views to work) and this particular issue (moving the admin code into a .inc file) has now been fixed. The other issues will need to be tackled separately. If you don't want to submit patches against the new code I will take a look at your code and pick out one change at a time. I will create new issues for these.

pasqualle’s picture

Ok, I will post my code with your changes in it, in a new issue.

pasqualle’s picture

Priority: Critical » Normal

Title: separate the admin pages +other fixes and improvements » separate the admin pages
Priority: Critical » Normal
Status: Needs review » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.