Page MenuHomeIn-Portal Phabricator

INP-1473 - Add "edit_field" parameter to "AdminEditButton" tag
ClosedPublic

Authored by erik on Sep 4 2015, 5:03 AM.

Details

Summary

Added ability to specify field name (that is present on editing template) that needs to be in focus when editing template opens.

Test Plan
NOTE: Test plan written for 21.5'' monitor. If your monitor is larger make window smaller by height to perform the test.

Preparations

  1. go to Admin Console
  2. go to Website & Content & Structure & DataDirectory section
  3. click on Sections tab
  4. create Sub Directory category in it using New Section button on toolbar
  5. go to Website & Content & Structure & DataDirectorySub Directory section
  6. click on Links tab
  7. add a link using New Link button on toolbar
  8. make following changes in IDE in advanced theme templates:
    • add edit_field="FormSubmittedTemplate" attribute to <inp2:st_EditPage mode="start" item_prefix="$item_prefix"/> tag in platform/designs/default_design.des.tpl template
    • add edit_field="Url" attribute to <inp2:AdminEditButton/> tag in in-link/elements/links.elm.tpl template

Test Plan

NOTE: Repeat all steps below replacing Same Window with Popup Window and Modal Window.
  1. go to Admin Console
  2. select Same Window value for Editing Window Style variable in ConfigurationWebsiteAdvanced section
  3. go to Website & ContentBrowse Website section
  4. click on Content Mode button in top frame
  5. click on the green Section Properties button in the right corner
  6. confirm that edit window, that opens is scrolled automatically to Online Form Submitted Template field
  7. go to Website & ContentBrowse Website section
  8. click on Content Mode button in top frame
  9. use top menu to go to DirectorySub Directory section
  10. confirm, that link created before is shown in there
  11. click on green Edit Item button next to that link
  12. confirm that editing window was opened and was automatically scrolled to URL field

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.3.x
Lint
Lint ErrorsExcuse: Full files re-formatting is not part of this task.
SeverityLocationCodeMessage
Errorcore/kernel/db/db_event_handler.php:1816PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/kernel/db/db_event_handler.php:1819PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/kernel/db/db_event_handler.php:1821PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/kernel/db/db_event_handler.php:1822PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/admin/admin_tag_processor.php:1221PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/admin/admin_tag_processor.php:1230PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/admin/admin_tag_processor.php:1238PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/admin/admin_tag_processor.php:1240PHPCS.E.CodingStandard.Classes.ClassDeclaration.SpaceBeforeCloseBraceCodingStandard.Classes.ClassDeclaration.SpaceBeforeCloseBrace
Errorcore/units/admin/admin_tag_processor.php:1240PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Unit
No Unit Test Coverage
Build Status
Buildable 452
Build 452: arc lint + arc unit

Event Timeline

erik updated this revision to Diff 300.Sep 4 2015, 5:03 AM
erik retitled this revision from to INP-1473 - Add "edit_field" parameter to "AdminEditButton" tag.
erik updated this object.
erik edited the test plan for this revision. (Show Details)
erik added a reviewer: alex.
erik added 1 JIRA issue(s): INP-1473.
alex requested changes to this revision.Sep 16 2015, 10:38 AM
alex edited edge metadata.
alex added inline comments.
core/admin_templates/incs/footer.tpl
79–96

Move all this code at the end of form_error_warning block definition.


The prefix block parameter available in form_error_warning block would allow to universally identify edited unit.

82

Please replace with:

var $row = get_control('<inp2:$prefix_InputName name="#FIELD_NAME#" js_escape="1"/>', '<inp2:m_Get name="edit_field"/>', 'row');

See comment related to PassedPrefixTag declaration.

82–84

Element query code & IF statement needs to be placed inside hook code.


The m:OnAfterFormInit hook is called after Form class wraps elements in <div id="scroll_container"> DIV in another DIV used for scrolling. Nature of wrapping operation is performing copy/paste of elements in that DIV and reference to DOM element stored in $row variable would point to different element at that point.

91
  1. replace parent.frames['main'] with getFrame('main')
  2. use same scrollTop method, but this time on the document object

  1. the proper method for the job must be used
  2. I see no reason for using 2 different functions for scrolling
core/kernel/db/db_event_handler.php
1819

Please replace with:

if ( strlen($value) ) {

The 0 in $value variable won't result in parameter being passed in upcoming redirect. Right now it won't create any problem, but if in the future more $pass_through variables would be used, then this won't create problems for them.

core/units/admin/admin_tag_processor.php
1222–1237

Please delete this.


The prefix guessing logic doesn't cover the case, when there are multiple prefixes in url and one, that is used on the form isn't last passed.

This revision now requires changes to proceed.Sep 16 2015, 10:38 AM
erik added inline comments.Sep 23 2015, 4:54 AM
core/admin_templates/incs/footer.tpl
91

scrollTop on the document object not always working - this solution is not cross-browser.

alex added inline comments.Sep 23 2015, 5:31 AM
core/admin_templates/incs/footer.tpl
91

Can you please be more specific about browsers, where this doesn't work?


On both Chrome and Firefox I did this:

  • opened ConfigurationAdvanced section in Admin Console
  • typed following in FireBug / Console:
window.frames['main'].$(window.frames['main'].document).scrollTop(10);
  • it worked
erik updated this revision to Diff 377.Oct 16 2015, 5:24 AM
erik edited edge metadata.

Improved js code for non-popup case.

erik added inline comments.Oct 16 2015, 5:26 AM
core/admin_templates/incs/footer.tpl
91

Yes, it works. Seems, i used some wrong approach when trying scrollTop. I do not remember what code was tried before scrollBy solution.

alex edited edge metadata.Mar 10 2016, 5:09 AM
alex added a project: Restricted Project.
alex requested changes to this revision.Apr 5 2016, 4:21 AM
alex edited edge metadata.
alex added inline comments.
core/admin_templates/incs/footer.tpl
79–96

After discussing over Skype decided not to do this, because the form_error_warning block is only shown when there is an error on the form.

82

Decided not to do this.

82–84

It appears, that this change wasn't done in 377 diff.

91

It appears, that this change wasn't done in 377 diff.

core/kernel/db/db_event_handler.php
1819

It appears, that this change wasn't done in 377 diff.

core/units/admin/admin_tag_processor.php
1222–1237

Decided not to do this.

Please update test plan to confirm, that it works for non-categories (e.g. products) as well.

This revision now requires changes to proceed.Apr 5 2016, 4:21 AM
erik edited the test plan for this revision. (Show Details)Apr 5 2016, 6:11 AM
erik edited edge metadata.
erik updated this revision to Diff 516.Apr 5 2016, 6:25 AM
erik edited edge metadata.
erik edited the test plan for this revision. (Show Details)

Made requesred improvements.

alex edited the test plan for this revision. (Show Details)Apr 5 2016, 9:07 AM
alex edited edge metadata.
alex requested changes to this revision.Apr 5 2016, 9:10 AM
alex edited edge metadata.

I've enhanced test plan with more details and tested that all works according to plan.

Please make these minor changes please.

core/admin_templates/incs/footer.tpl
84

Please also add js_escape="1" to the m_Get tag call.

90

Please also add js_escape="1" to the m_Get tag call.

core/units/admin/admin_tag_processor.php
1232–1233
  1. please change to:
$passed_prefixes = explode(',', $this->Application->GetVar('passed'));
$last_prefix = array_pop($passed_prefixes);
  1. use $last_prefix variable below instead of $prefix
1235

Remove tag parameter from $params array before calling tag from $params['tag'].


The tag parameter is passed to called tag, but that tag doesn't know what to do with it.

This revision now requires changes to proceed.Apr 5 2016, 9:10 AM
erik updated this revision to Diff 517.Apr 5 2016, 9:22 AM
erik edited edge metadata.

Made new requested changes.

erik updated this revision to Diff 518.Apr 5 2016, 9:25 AM
erik edited edge metadata.

Removed unnecessary links.elm.tpl change.

alex accepted this revision.Apr 5 2016, 9:28 AM
alex edited edge metadata.
This revision is now accepted and ready to land.Apr 5 2016, 9:28 AM
alex requested changes to this revision.Jul 25 2016, 4:19 AM
alex edited edge metadata.
alex added inline comments.
core/admin_templates/incs/footer.tpl
79–96

Please move JavaScript code block up in the DOM tree to be last node inside <body> tag.


Having JavaScript code after </html> tag isn't guaranteed to work in all Web Browsers.

This revision now requires changes to proceed.Jul 25 2016, 4:19 AM
erik updated this revision to Diff 588.Aug 4 2016, 4:52 AM
erik edited edge metadata.

New code moved into "body" tag.

alex accepted this revision.Nov 21 2016, 2:59 AM
alex edited edge metadata.
This revision is now accepted and ready to land.Nov 21 2016, 2:59 AM
This revision was automatically updated to reflect the committed changes.