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 html_* functions into Ui class member using Form\Form class #3198

Merged
merged 78 commits into from Oct 13, 2020

Conversation

ssahara
Copy link
Collaborator

@ssahara ssahara commented Jul 7, 2020

This PR contains new Ui\Editor class that will replace html_edit() defined in inc/html.php file.
Some html_* functions that display HTML forms, in other words UI, may be refactored using new Form\Form class and implemented as a Ui class member. Is this a right way for further work?

no space between printed items such as date, unsername, sizechange info. Further work needed for better looks/design.
no space between printed items such as date, unsername, sizechange info. Further work needed for better looks/design.
The code for page navigation in Ui\Revisions is different with Ui\Recent.
fix code sniffer errors (method name is not in camel caps format)
inc/html.php Outdated Show resolved Hide resolved
$form is object, therefore it is always treated  as reference without '&'.
Ui\Diff()::diffNavigation() still uses Doku_Form class, Further work needed!
NOTE: necessary to inhibit prefillInput.  Form\DropdownElement::toHTML() calls val(), which calls val('') during prefillInput(), will cause wrong selected option.
call dbg_deprecated(), even this function lost compatibility because its argument  $param['form'] has changed to hold Form\Form object
@Klap-in
Copy link
Collaborator

Klap-in commented Sep 8, 2020

I have added the event HTML_EDIT_FORMSELECTION also to the wiki page, due to the form change. No other event included a Doku_Form so far I could see. Or do you know others? Thanks for adding the deprecations as well!

to prevent breaks in old   HTML_EDIT_FORMSELECTION event handler, such as edittable plugin
encourage plugin devs to use new Form class
@ssahara
Copy link
Collaborator Author

ssahara commented Sep 11, 2020

@Klap-in , the event HTML_EDIT_FORMSELECTION has changed to EDIT_FORM_ALTERNATE to prevent breaks in relevant plugins, most likely edittable.
Also, html_form() and html_horm_output(), that use Doku_Form, calls dbg_deprecated().

inc/html.php Show resolved Hide resolved
@Klap-in
Copy link
Collaborator

Klap-in commented Sep 11, 2020

https://www.dokuwiki.org/devel:events_list#naming_structure
The name convention asks for the third part to describe the default action. I do not know a good naming alternative, so if you do not know as well then others can better help with suggestions.

@Klap-in
Copy link
Collaborator

Klap-in commented Sep 23, 2020

The old and the new event name i.e. HTML_EDIT_FORMSELECTION and EDIT_FORM_ALTERNATE are both unclear for me.

Then name convention asks: <location>_<event_data>_<action_or_state>
What do you think about: EDIT_FORM_ADDTEXTAREA

(for me that is the final remark on this PR ;-) )

change event name to prevent breaks in old HTML_EDIT_FORMSELECTION or early proposed EDIT_FORM_ALTERNATE event handler, such as edittable plugin
@Klap-in Klap-in self-requested a review September 27, 2020 06:35
Copy link
Collaborator

@Klap-in Klap-in left a comment

Choose a reason for hiding this comment

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

Thanks for your work!

@ssahara
Copy link
Collaborator Author

ssahara commented Oct 10, 2020

Event Descriptions for new FORM_*_OUTPUT in dev manual has updated!

@Klap-in
Copy link
Collaborator

Klap-in commented Oct 12, 2020

For me this is fine to merge. Fine to go?

@splitbrain splitbrain merged commit 0afbc17 into dokuwiki:master Oct 13, 2020
@splitbrain
Copy link
Collaborator

@ssahara awesome work! thank you!

@dregad
Copy link
Contributor

dregad commented Jan 26, 2021

Is there a reason why this is an anonymous class? Can we turn it into a proper class in it's own file?

Funny, I came here to ask the same question 😉

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

5 participants