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

More robust template path hints (block name, block parents #571

Closed
erikhansen opened this issue May 16, 2014 · 40 comments
Closed

More robust template path hints (block name, block parents #571

erikhansen opened this issue May 16, 2014 · 40 comments
Assignees
Labels
improvement Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development up for grabs

Comments

@erikhansen
Copy link
Contributor

Currently, the "template path hint" feature displays the template name and block class name.

Fabrizio Branca's Aoe_TemplateHints module extends the native Magento 1.x template path hint functionality to add much more information to the hints including:

  • Block name (very useful)
  • Parent block names
  • Module
  • Caching status

Here is a screenshot from his blog post, visualizing what this could look like:
jpeg

I propose that this functionality be built into Magento 2. If Magento accepts this proposal, I may be willing to build this functionality and then issue a pull request to have it incorporated into Magento 2.

@erikhansen
Copy link
Contributor Author

@fbrnc Would you be willing to change the license type of Aoe_TemplateHints to MIT to allow code from that module to be ported to Magento 2? You've done a great job with this module and I'd like to be able to reference and port code from that module.

@verklov
Copy link
Contributor

verklov commented May 27, 2014

@erikhansen, thank you for the idea! The PO responsible for this area will look at your suggestion and we will figure out what to do. Anyway, the ticket is in the product backlog now. Let's see what we can do about the suggestion you are offering.

@erikhansen
Copy link
Contributor Author

@verklov Thanks. I would be interested in contributing to the development of this, if it is something that Magento would be willing to include in the core.

@tkacheva
Copy link

@erikhansen Thank you for the great idea! I will include it into our roadmap

@erikhansen
Copy link
Contributor Author

@tkacheva If I worked on this feature and submitted a pull request, would it get included into Magento 2?

@mhhansen
Copy link
Contributor

👍

@tkacheva
Copy link

tkacheva commented Jun 2, 2014

@erikhansen Yes, for sure if it passes code review and meets our license policy

@vpelipenko
Copy link
Contributor

Internal ticket: MAGETWO-24798

@fbrnc
Copy link

fbrnc commented Jan 29, 2015

I must have missed this ticket in the first place. Yes, I'd totally love to see a template hints like the ones I implemented for Aoe_TemplateHints in Magento 2 and I'd also be willing to contribute that code.
One of the important benefits of Aoe_TemplateHints over the native ones is that Aoe_TemplateHints does not only show blocks inheriting from Mage_Core_Block_Template, but all blocks (inheriting from Mage_Core_Block_Abstract). That's an important detail :)

@alankent
Copy link

alankent commented Feb 2, 2015

I checked internally, only to be reminded that this was already approved above by @tkacheva mid last year. So if someone is willing to work on this (based on AOE code as starting point - thanks Fabrizio!), could interested parties leave a note on this issue to coordinate work between you.

@vpelipenko vpelipenko added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Apr 21, 2015
@ryanstreet
Copy link

I would be willing to work on this issue. Is it still up for grabs?

@erikhansen
Copy link
Contributor Author

@ryanstreet yes, it is still up for grabs, as far as I know. It would be great if you wanted to work on it.

@ryanstreet
Copy link

Awesome. I'm on it!

@piotrekkaminski
Copy link
Contributor

Great @ryanstreet

@vpietri
Copy link

vpietri commented Jun 30, 2015

Thanks @ryanstreet. Looking forward this great module in Magento 2.

@ryanstreet
Copy link

Your welcome!

magento-team pushed a commit that referenced this issue Sep 4, 2015
@erikhansen
Copy link
Contributor Author

I don't have time to work on this right now, but just pitched in a $25 bounty.

@ryanstreet
Copy link

Unless someone can get to it sooner, I have this fix slotted in my schedule for February 16. If someone else can get to it, by all means do so.

@erikhansen
Copy link
Contributor Author

@ryanstreet It would be awesome if you could take this on in February. Frontend developers will thank you for years to come. The Aoe_TemplateHints extension is pretty robust and does quite a bit more than what I think we could reasonably expect to be included in the Magento core.

If I were you, I would target adding this functionality to core Magento: In addition to the block class and template file, add the block name and block alias to the template hints.

I'm not sure if you've submitted any pull requests for Magento 2, but you'll need to keep in mind that Magento expects integration/unit tests to be included in any pull request. You can read up here on how to get Magento's native tests running on your development environment: http://davidalger.com/development/magento/running-the-magento-2-test-suite/ Since Magento should already have tests in place to verify the existing template hints functionality, it shouldn't be too difficult to expand upon those tests to cover the new functionality.

Here is my suggestion of how to change the config:

configuration___settings___stores___magento_admin

Here is a suggestion for how you might add the block name and class:

shopping_cart

You'll want to test with all combinations of settings and block combinations (e.g., blocks with no names but aliases, etc)

@alankent
Copy link

I would love to see this tool improved, or a better version created as a module. It just struggles to get high enough on the list compared to other features internally. Should we fix checkout or the hints too? Checkout! Should we fix payments integration or the hints tool? Payments! This sort of project I think would make a great community contribution.

@erikhansen
Copy link
Contributor Author

@alankent I'm glad you would like to see this tool improved but certainly understand the difficulty in prioritizing it against other features. I like that Magento has focused on more building developer tools (including a Grunt/livereload workflow, bin/magento, etc). I want to contribute in helping make the platform better for developers (via things like this PR).

Do you have any feature suggestions beyond the pared down functionality I recommended in my previous comment? Any time I've used Aoe_TemplateHints, the main thing I've wanted out of it is the ability to see block names/aliases. Although on occasion I've used the block nesting, caching status, and "displaying all of the information as HTML comments instead of HTML elements" features. I could see those features being useful, but not as much as just getting block names/aliases displaying.

@alankent
Copy link

I would be tempted to have all the hints in a popup box when you hover (just like the start of this thread). For small areas on the screen, all the text gets truncated at present. I would rather have a little clean popup box that can include a clear list of the settings. Then you can add more content as appropriate over time without running out of screen real estate.

You could then drop some of the options regarding what to display - just always display it all! You need to turn it off at present to make things easier to read at times.

It would be nice if the popup box markup is clearly separate from the normal HTML module output (e.g. in a separate div) so you can expand/collapse more easily when doing inspect element. At present, the hint markup is pretty in your face making "inspect element" pretty unfriendly when the hints are turned on.
image

I would love to see the path from the root of the view element tree with the names of all the blocks, or at least the block's parent, so you can work out how to construct layout instructions more easily. (If you got crazy, a full expand/collapse tree of all the blocks that make up the current page would be great! Then add the path name to the layout file the block came from... but I think that is going too far.) Just having hovers is a step forwards however as you can deduce the block tree structure just moving the cursor around the page.

I personally find the red not very nice - looks like an error rather than just a developer exploration tool. I do like the rectangles though - helps understand the area the block covers.

Again, if you wanted to go crazy, you could make the popup have a link to display the PHTML template file (not just show the path name). However this starts becoming a security concern as you are starting to make it easier to read the source code remotely, which I would frown upon.

@alankent
Copy link

I am not a JavaScript guy - I guess you could have bar at the top of the page with options, like an on/off button so you can just hover all around the page without inserting the rectangles. So you see what the page really looks like. Think Z-ray. But clearly that is a lot more work. Simply having popup boxes with little icons instead of embedding the text directly into the page would be a huge step forwards. E.g. draw a rectangle with a little space inserted, and a little box or triangle in corner that you click or hover over to display the popup.

@erikhansen
Copy link
Contributor Author

@alankent I love where you are going with this.

I like the developer bar idea. It reminds me of Alan Storm's Commerce Bug.

I agree with you that moving the information into a tooltip is the best way to unobtrusively display robust debugging information.

Having the popup link allow you to open the phtml file in your IDE is a great idea. PhpStorm supports the phpstorm://open?file=%f&line=%l format for opening files (coincidentally I just added these links to Xdebug in Classy Llama's development environment). TextMate has a similar link format, and I'm sure that Sublime, Netbeans, etc also do. We could add a dropdown in the "Debug" section allowing a developer to choose a list of IDEs from a dropdown.

I'm going to put some time into this over the coming week and see what I can come up with.

@alankent
Copy link

"I love where you are going with this." - if you get something built, then I would think the community would turn it around and point it back at you! ;-)

@aepod
Copy link
Contributor

aepod commented Jan 15, 2016

Drupal 8 has a pretty nifty built in toolbar(although it was a little tricky to get working). It's based on something in symfony and has quite a bit of templating and other info, as well as a click through to very detailed info page on some bits.

@erikhansen
Copy link
Contributor Author

@aepod Interesting. I'll have to install Drupal 8 locally and play around with it.

@alankent It looks like Fabrizio implemented support in Aoe_TemplateHints for the older style of linking to PhpStorm:
configuration___system___smocked_admin_-_magento_admin_smocked_auctions_smocked_auctions

@ryanstreet
Copy link

Wow! What great feedback!

@aepod I'll have to look at what Drupal 8 does with their hint system.

@alankent and @erikhansen: I think an iterative process will be the best way to handle this. I don't want to pack too much into it all at once, and have it take forever to complete. My initial thoughts are to get basically what Aoe_TemplateHints did in there first. I really like the idea of the PHPstorm popup feature. I think that would be good to add in a later iteration.

I have worked with the Laravel debugBar and the Symfony Debug Toolbar as well. I would like something like that in here as well. That might be beyond what we are trying to achieve with this particular request, but I think it would be a great addition. Even if it must go into another request, I think the developer community would get a lot of mileage out of a feature like that.

@erikhansen
Copy link
Contributor Author

@ryanstreet I just sent you an email about collaborating on this PR, as I've begun working on it, but would love to work on it together.

@OLTC-fperrin
Copy link

Hey, I found this thread by accident while working on a more advanced template hints module based on the work of @fbrnc which you can find here: Ltc_AdvancedTemplateHints. I'm sure it can be improved since I haven't been working with Magento 2 for long; any feedback is welcome.

@erikhansen
Copy link
Contributor Author

@LTC-fperrin I've started working on this functionality in a Magento 2 repo fork: https://github.com/erikhansen/magento2/tree/feature/better-template-hints I've implemented the basic config structure and have begun working on output of the more advanced template path hints. I'll be sure to reference your extension as I continue to work on this pull request. Thanks for sharing!

@alankent
Copy link

alankent commented Mar 1, 2016

Hi @erikhansen, just wondering how you were going. I have a volume 1 (ebook) on themes coming really soon now, just wondering if this is likely to hit the streets soon in which case I would mention it. (No pressure, just wond'rin!) - I can always leave to vol 2.

@erikhansen
Copy link
Contributor Author

@alankent I've not been able to put as much time into this as I would have liked, so I'm afraid you'll have to cover it in volume 2. I'm looking forward to reading your book!

@piotrekkaminski
Copy link
Contributor

Related issues:
MAGETWO-24837 Extended information in "template path hint"
MAGETWO-24840 Improvement: template path hint in tooltip

@erikhansen
Copy link
Contributor Author

@piotrekkaminski I know that @jonathanhodges and @beejhuff worked on an extension to implement some of this functionality at the Imagine Hackathon. I just talked with @jonathanhodges and he said that refactoring would need to be done to the extension before merging in any of the code, but it is at least a helpful reference point: https://github.com/magento-hackathon/magento2-improved-template-hints

The next steps should be to merge their code into the code I've been working on and submit this as a PR. However I'd like to know more about those internal issues because if the Magento core team is already working on this, I'd like to know what they're planning on doing as there is likely overlap.

@piotrekkaminski
Copy link
Contributor

@erikhansen the issues are unscheduled, in backlog for now. So if you have can do anything to contribute, that would be great.

@ryanstreet
Copy link

That's great. Thank you @piotrekkaminski .

@tkacheva
Copy link

@piotrekkaminski Added both tickets to DX backlog.

@piotrekkaminski
Copy link
Contributor

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

@paales
Copy link
Contributor

paales commented Nov 25, 2016

okorshenko pushed a commit that referenced this issue Feb 7, 2017
Fixed issue: 
- MAGETWO-60145: Mainline Build Failure for Sample Data Tests
magento-engcom-team added a commit that referenced this issue Apr 16, 2019
 - Merge Pull Request magento/graphql-ce#571 from magento/graphql-ce:place-order-guest
 - Merged commits:
   1. 241450e
   2. 05f3458
   3. 87312d4
magento-devops-reposync-svc pushed a commit that referenced this issue Dec 4, 2023
small improvements to ACPT-1587 & ACPT-1584
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development up for grabs
Projects
None yet
Development

No branches or pull requests