| opendevreview | chandan kumar proposed openstack/watcher-dashboard master: Improve audit creation model and add text wrapping https://review.opendev.org/c/openstack/watcher-dashboard/+/963778 | 10:27 |
|---|---|---|
| sean-k-mooney | chandankumar: woudl you mind uploading updated images fo the final version. | 10:38 |
| sean-k-mooney | chandankumar: my devstack kind fo broke because the disk filled up so i need to recreated a new env | 10:39 |
| sean-k-mooney | but i wont have time to do that today | 10:39 |
| chandankumar | sean-k-mooney: sure | 10:42 |
| chandankumar | sean-k-mooney: https://i.imgur.com/R2ATF7C.png | 10:43 |
| sean-k-mooney | that is without the applicaiton fo the with correct? | 10:44 |
| sean-k-mooney | the modal-xl class need to be applied to the div with modal-dialog | 10:45 |
| sean-k-mooney | in that image its still resticted to ~1/3 of the space not 95% of the width | 10:46 |
| sean-k-mooney | the wrapping appares to be fixed which is good | 10:47 |
| chandankumar | yes, it is not using the whole space | 10:47 |
| chandankumar | I am not sure modal xl class is applied | 10:48 |
| chandankumar | need to debug that | 10:48 |
| sean-k-mooney | ack | 10:48 |
| sean-k-mooney | i think instead of {% extends "horizon/common/_modal_form.html" %} you may need to duplicate its content and apply that class to the root modal div | 10:49 |
| sean-k-mooney | i have not checked to closely | 10:49 |
| sean-k-mooney | you need to apply it to this div https://github.com/openstack/horizon/blob/4b81bd78fc22b846c82b18fc70bec5b49163f514/horizon/templates/horizon/common/_modal.html#L5 | 10:50 |
| sean-k-mooney | i think | 10:50 |
| sean-k-mooney | you currently tryign to set it on https://github.com/openstack/horizon/blob/4b81bd78fc22b846c82b18fc70bec5b49163f514/horizon/templates/horizon/common/_modal.html#L1 right | 10:51 |
| sean-k-mooney | that might work also but the div with modal-dialog was the one that i saw in firefox that was settign the width | 10:51 |
| opendevreview | Joan Gilabert proposed openstack/watcher master: Fix check for volume type in cinder helper migrate https://review.opendev.org/c/openstack/watcher/+/963857 | 10:54 |
| sean-k-mooney | we can see an example of this used here https://github.com/openstack/horizon/blob/4b81bd78fc22b846c82b18fc70bec5b49163f514/openstack_dashboard/contrib/developer/static/dashboard/developer/theme-preview/theme-preview.html#L1644 | 10:59 |
| chandankumar | sean-k-mooney: instead of model-xl div, I used minimal javascript $('#create_audit_form').closest('.modal-dialog').addClass('modal-xl'); (Find the form and find the parent box and make it wide) then it appears like this https://i.imgur.com/MEGwGqC.png, It will avoid duplicating the stuff or I can duplicate the horizon model form content | 11:11 |
| sean-k-mooney | that coudl work as a short term solution | 11:12 |
| sean-k-mooney | its proably ok for now but we shoudl follwo up with horizon to suppot settign the class via a template block | 11:12 |
| sean-k-mooney | im ok with the js solution for this bug as a temperoty solution | 11:13 |
| sean-k-mooney | making the class settable is a really small chang to the horizon template | 11:13 |
| sean-k-mooney | so we should idealy also do that and submit a follow up to replace teh js solution with that | 11:13 |
| sean-k-mooney | also that looks much better | 11:14 |
| sean-k-mooney | chandankumar: if you push up that version we can do the oneline change to https://github.com/openstack/horizon/blob/master/horizon/templates/horizon/common/_modal.html#L5 to supprot passign extra classes and a follow up to the plugin | 11:19 |
| sean-k-mooney | that way we can backport the js version without needing to backprot the horizon change | 11:19 |
| sean-k-mooney | and we can clanup the plugin on master once the horizon change is landed | 11:19 |
| chandankumar | I am trying to duplicate it in watcher-dahsboard now | 11:20 |
| opendevreview | chandan kumar proposed openstack/watcher-dashboard master: Improve audit creation model and add text wrapping https://review.opendev.org/c/openstack/watcher-dashboard/+/963778 | 11:33 |
| chandankumar | sean-k-mooney: ^^ screenshot: https://i.imgur.com/v73cRG6.png | 11:33 |
| chandankumar | with duplicated code | 11:34 |
| chandankumar | hmm | 11:35 |
| chandankumar | for current backport, going with js stuff sounds good | 11:36 |
| sean-k-mooney | hum im surpised that worked | 11:36 |
| sean-k-mooney | im not sure this is a good approch as it will change the defintion for everyone i think | 11:36 |
| chandankumar | in that code, go with the duplicated one? | 11:36 |
| chandankumar | s/code/case | 11:36 |
| sean-k-mooney | when i suggestign vendoring i explcity ment using a diffent name so that it woudl not overed the default version form hoizon | 11:37 |
| sean-k-mooney | which is why i said aw would have to vendor the modal form as well | 11:37 |
| chandankumar | let me change the name | 11:37 |
| sean-k-mooney | to change the name you will need to also vendor _modal_form.html | 11:38 |
| sean-k-mooney | and have it refecne that new name | 11:38 |
| sean-k-mooney | the render looks good to me however | 11:38 |
| sean-k-mooney | so if we fix those two thing im fine with this solution until we update horizon | 11:38 |
| opendevreview | chandan kumar proposed openstack/watcher-dashboard master: Improve audit creation model and add text wrapping https://review.opendev.org/c/openstack/watcher-dashboard/+/963778 | 11:51 |
| chandankumar | sean-k-mooney: updated it with different name | 11:52 |
| chandankumar | screnshot: https://i.imgur.com/lf0wfp6.png | 11:53 |
| sean-k-mooney | hum so the only other comment i have is when you renamed it you shoudl have moved it out of the horizon namespace | 11:55 |
| sean-k-mooney | so watcher_dashboard/templates/horizon/common/_modal_xl.html shoudl be watcher_dashboard/templates/_modal_xl.html | 11:56 |
| sean-k-mooney | os somethign tha tis prefixed with our plugin in some wway | 11:56 |
| sean-k-mooney | ah it shoudl be in here https://github.com/openstack/watcher-dashboard/tree/master/watcher_dashboard/templates/infra_optim | 11:57 |
| sean-k-mooney | chandankumar: if you would not mind moving it to there im +2 on it otherwise | 11:58 |
| sean-k-mooney | same for watcher_dashboard/templates/horizon/common/_modal_form_xl.html | 11:58 |
| sean-k-mooney | we shoudl proably move or delete https://github.com/openstack/watcher-dashboard/blob/master/watcher_dashboard/templates/horizon/common/_items_count_domain_page_header.html too as i think overrided horizon templeates liek this is generally not someithng a plugin shoudl do | 11:59 |
| sean-k-mooney | but that a prexisting issue and should be fixed sepreatly | 11:59 |
| chandankumar | will do that | 12:11 |
| chandankumar | yes, correct, I will drop page_header.html seperately | 12:16 |
| opendevreview | chandan kumar proposed openstack/watcher-dashboard master: Improve audit creation model and add text wrapping https://review.opendev.org/c/openstack/watcher-dashboard/+/963778 | 12:19 |
| chandankumar | Done! | 12:19 |
| chandankumar | screenshots: https://i.imgur.com/GL6gEdM.png and https://i.imgur.com/MMRld0r.png | 12:20 |
| chandankumar | sean-k-mooney: thank you for me on improving this patch :-) | 12:21 |
| sean-k-mooney | +2 those look good to me | 12:23 |
| sean-k-mooney | if you have time next sprint woudl you mind trying to upstream the chagne to horizon | 12:23 |
| chandankumar | sean-k-mooney: sure, will do that | 12:45 |
| *** haleyb|out is now known as haleyb | 12:59 | |
| jgilaber | sean-k-mooney, when you have sometime please review again https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/958644 I think I addressed all your comments | 13:08 |
| jgilaber | chandankumar, please also add to your review list ^^ this patch and the next ones in the series | 13:09 |
| jgilaber | thanks! | 13:09 |
| *** gmaan_pto is now known as gmaan | 16:00 | |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Move decision-engine monitoring service to the decision-engine https://review.opendev.org/c/openstack/watcher/+/963252 | 16:01 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: APISchedulingService migrate audits also on first discovery of services https://review.opendev.org/c/openstack/watcher/+/963981 | 16:01 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!
