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

Feature/js modal rev #194

Closed
wants to merge 27 commits into from
Closed

Conversation

ibelar
Copy link
Contributor

@ibelar ibelar commented Jul 7, 2017

Javascript and Modal Revision

Change reason

Learning semantic-ui furthermore allow me to better understand the semantic module settings and behaviours. These changes will take more advantage of semantic-ui functionality now and in future. For example, modal are now using semantic api call in order to load their dynamic content. By using a service to control them, we can also implement our own behaviour to them. Loading content to api module also allow for better error control. Finally, modal can now be add to DOM directly using the new Modal.php class.

Javascript

Folder structure

Better structure the javascript file by making them more modular. The folder structure is as follow

  • plugins: contains the jquery plugin file
  • helpers: contains extended jquery function, like $.addParams, perhaps eventually univ()...
  • service: contains mainly singleton class responsible to manage semantic-ui module, like the api or modal module throughout the entire app.

jQuery plugin enhancement

Better handling of jQuery plugin creation via plugin.js. Also now handle direct plugin function call using $('element').plugin('functionToCall', [param]) type syntax.

Service class

Two new service classes, ApiService and ModalService, for managing api or modal throughout the app.

  • ApiService: better handling of error message and api call done via other service, like modal;
  • ModalService: better handling of modal
    • allow support for multiple modal, properly handling close trigger event.
    • allow dynamic content via createModal or Modal.php.
    • allow for regular content via Modal.php

Service can be add for other semantic-ui module like form, tab etc, in future.

PHP

Modal.php

Add Modal class in order to allow modal use within a page.

This is an example where clicking a button will add a modal that contains another button that will load a CRUD with full editing capability in modal as well.

$modal1 = $layout->add('Modal');
$modal2 = $layout->add('Modal');

$vp = $layout->add('VirtualPage'); // this page will not be visible unless you trigger it specifically
$vp->add(['Header', 'Contens of your pop-up here']);
$vp->add(['LoremIpsum', 'size'=>2]);
$vp->add('Button')->set('Proceed')->on('click', $modal2->js()->modal('show'));

$vp1 = $layout->add('VirtualPage'); // this page will not be visible unless you trigger it specifically
$c = $vp1->add('CRUD');
$c->setModel($m)

$modal1->setUri($vp->getURL('cut'));
$modal2->setUri($vp1->getURL('cut'));

$bar = $layout->add(['View', 'ui'=>'buttons']);
$b = $bar->add('Button')->set('Load Modal');

$b->on('click', $modal1->js()->modal('show'));

ibelar added 10 commits July 4, 2017 09:23
- add possibility to simply use html in jsModal, not only Json;
- redesign plugin implementation via a plugin base class
  - add possibility to create plugin method call
  - plugin creation properly done
- move addParam as an helper function.
@romaninsh
Copy link
Member

My other comment is that you should add a demo page for this new functionality.


namespace atk4\ui;

class Modal extends View
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering, perhaps Modal should extend Virtual Page?

$modal = $layout->add(['Modal', 'modal title go here']);

$layout->add('Button')->on('click', $modal->js()->modal('show'));

$modal->add('LoremIpsum'); // already work fine since it's virtualpage.

Copy link
Contributor Author

@ibelar ibelar Jul 10, 2017

Choose a reason for hiding this comment

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

I can look into that. The idea of using Modal as a regular view was to conform with semantic modal behaviour, i.e. adding the content in DOM.

$modal = $layout->add(['Modal', 'modal title go here']);
$msg = $modal->add([
    'Message',
    'This is a title of your message',
]);
$msg->text->addParagraph('You can add some more text here for your messages');

$layout->add('Button')->on('click', $modal->js()->modal('show'));

Also saving a trip to server for simpler modal need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here the things with Modal as a VirtualPage. Correct me if I am wrong but VP does not output any content to DOM until they are called and $modal->js()->modal('show') need to have the $modal element in place in DOM for it to work properly.
Which mean that if we put Modal as a derived class of VirutalPage, we will need to add the modal html element to the DOM prior to call modal('show'), which mean using createModal.js plugin again and this is already use by jsModal.php.

What if we change the method name to

$modal->addVirtualPage($vp) 

instead of

$modal->setUri($vp->getUrl('cut'))

We could do the proper wiring of setUrl inside AddVirtualPage method and it would probably make more sense to developers using it.

Let me know your thoughts

Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking about making it transparent, e.g.

$modal->set(function($page) {
   $page->add('LoremIpsum');
});

This way you can set callback for a modal and you don't have to create virtual page. One option is to extend VirtualPage (then set() wolud work) but you can also simply define function set().

@romaninsh
Copy link
Member

I have merged your other PR and conflicts appeared.

ibelar added 14 commits July 10, 2017 08:27
# Conflicts:
#	js/src/atk4-semantic-ui.js
#	js/src/plugins/createModal.js
…re/js-modal-rev

# Conflicts:
#	js/src/atk4-semantic-ui.js
#	js/src/plugins/createModal.js
Plugin.js : needed to pass option to main function. Because calling the
plugin twice with different option on the same element was not working
since old option was still set in element.
Other plugin file: Need to adjust main function to accept options
spinner.js: add a function in order to remove spinner from element.
When calling the reloadView more than once on the same element the
element was growing because of spinner content not remove.
reloadView: adjust plugin to remove spinner on element being reload.
One day I will get it right on the first commit….
@romaninsh
Copy link
Member

romaninsh commented Jul 14, 2017

Ok, nice cleanup and a lot of changes. Should be a good time to refactor, going into 1.2 branch, but looks like a lot of new stuff which has absolutely no documentation.

I wouldn't be in the position to document any of this, can I ask you to include documentation in .rst explaining how to use the new services?

The description of this PR is a good start, but please include some description and examples for all methods of new classes.

@ibelar ibelar closed this Jul 18, 2017
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

2 participants