feat: refinement of knowledge panel rendering template. - #9890
feat: refinement of knowledge panel rendering template.#9890deveshidwivedi wants to merge 17 commits into
Conversation
github-actions
Bot
added
📖 Knowledge Panels
❌ 17 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
|
Thank you @deveshidwivedi ! Could you separate the 2 changes (1: moving the CSS from the template to classes, and 2: how to see where the panel end)? It will be easier to merge the first 1 first. For the left border, the idea was to show where the body of the panel ends (when the accordion is opened), so that's it's clearer what happens when the user closes the panel. So the border should be on the panel title + content:
But we will have to try different ways, maybe it will look very busy with the border, and other solutions might be better. |
|
I have removed the border from this PR as requested, thank you.
I am sorry to misunderstand, I now got how it was supposed to be, we could try out different styles before finally making a PR to address this.
Sure! |
|
Hi @deveshidwivedi , have you been able to run your changes locally? I tried it but there's an issue with the icon image of all panels:
|
|
Hi @stephanegigandet! I have tried to correct the image icons, I had tried to run the changes locally but mistakenly made unrequired changes to the branch. Please have a look. Thank you. |
Hi @deveshidwivedi , there are still some typos that prevent some CSS rules to work. In order to make sure everything works, it would be good to test the product pages side by side: one side with your changes, and one side with the "main" branch, to check if you get visually similar results. |
|
Hi! @stephanegigandet I did run make front_build and make restart successfully, I see similar knowledge panels on both the localhost and the website. Could you please confirm? Thank you! |
|
@deveshidwivedi Thank you, it looks good! There is a conflicting change: Can you move the "z-index: 1;" to the customer-container scss, and resolve the merge conflict? |
|
@stephanegigandet does this look fine? |
|
Updating the branch to solve the failing tests (en:Cheese) |
stephanegigandet
left a comment
stephanegigandet
left a comment
There was a problem hiding this comment.
Looks good! Thank you!
|
@deveshidwivedi can you resolve the conflict ? |
|
Resolved! @alexgarel |
|
|
Hi @stephanegigandet, please take a look at this. The merge conflicts are now resolved. Thank you. |
|
damn, merge conflict again @deveshidwivedi 😅 |
…nd main features Merge origin/main into simpler-template. Two conflicted paths; everything else merged cleanly and is staged as-is. Context of each side: - This PR (simpler-template) de-inlines presentation from the knowledge panel template: templates/web/panels/panel.tt.html no longer carries style="..." attributes; the corresponding rules were moved into scss/_product-page.scss as reusable classes (.panel_small, .panel_margin_small/.panel_margin_large, .panel_title_small, .panel_icon very_small/small/large, .small-panel, .small-subtitle, .text-icon, .text-container, .valign-middle, .custom-class, .custom-container, .image_source, .hidden). - main kept the inline styles but functionally extended the same areas of panel.tt.html: panels without elements now render an inactive accordion item (new has_elements variable, accordion-navigation-inactive class, <a> switched to <div> via IF has_elements); new title fallbacks (title -> name) plus new value_string / value (with percent type) subtitle branches; image imgid lookup and nutripatrol_url parameter; actions wrapped in <div>; table_id renamed to element.id; map block rewritten as an ES module import. scss/_product-page.scss: - Conflict was two independent additions at the end of the file. Resolution keeps both blocks verbatim: first the knowledge panel classes from this PR (required by the refactored template - dropping them would leave every panel size/icon/text rule undefined), then main's SVG barcode responsive rules (#barcode_div_code/#barcode_div_svg inline-block, hidden under 600px). No selector or declaration was modified; only the three conflict marker lines were removed. templates/web/panels/panel.tt.html: - wrapper_for_other_types header: applied main's functional changes on top of the PR's class-based markup. The <ul> keeps the panel_margin_[small|large] class instead of main's inline margins; the <li> gains main's accordion-navigation-inactive when has_elements is false; the title tag uses main's <a>/<div> switch while keeping the PR's panel_title_small class instead of main's inline padding style. - subtitle block: kept the PR's class-based span for the existing subtitle branch, and grafted main's new value_string / value / percent branches onto it. Their inline style="font-size:0.9rem;" were expressed through the PR's equivalent small-subtitle class so that new content introduced by main follows this PR's stated goal (no inline styles in templates). Trailing whitespace from main's line was dropped. - Everything outside the conflict regions is the auto-merge result: main's has_elements logic, name/title fallback, value_string/value branches, image handling, action wrappers, table id and module-based map are all retained unchanged. Verification: no conflict markers remain; Template::Toolkit directive counts of the resolved file (opens 65 / ends 67) match main's side exactly, with the same pre-existing +2 comment-banner offset already present in both parents, so no block was broken by the resolution. Merge conflict resolved by Ox Alpha Free (Unlimited), running via OpenCode Zen. Co-authored-by: hangy <hangy@hangy.de>
github-actions
Bot
added
💥 Merge Conflicts
|
5 participants
What
Replaced inline styles with CSS classes for easier modification and better adaptability.
Related issue(s) and discussion