Skip to content

Conversation

mattkrupnik
Copy link
Contributor

@mattkrupnik mattkrupnik commented Sep 16, 2020

Description (*)

Actualy we are able to add custom attributes for links in customer account navigation via XML for example:

    <referenceBlock name="customer-account-navigation-account-link">
        <arguments>
            <argument name="attributes" xsi:type="array">
                <item name="class" xsi:type="string">account-custom-class</item>
            </argument>
        </arguments>
    </referenceBlock>

But when customer-account-navigation-account-link is as current page in this case it is https://sample.com/customer/account/ our custom attribute dosn't show because method getAttributesHtml() isn't used for current link.

For non-current link method getAttributesHtml() is used.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

Resolved issues:

  1. resolves [Issue] Pass custom attributes to current link via XML #38500: Pass custom attributes to current link via XML

Add possibility to pass custom attributes from XML to current link.
@m2-assistant
Copy link

m2-assistant bot commented Sep 16, 2020

Hi @mattkrupnik. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE,
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

@mattkrupnik
Copy link
Contributor Author

@magento run all tests

Copy link
Contributor

@rogyar rogyar left a comment

Choose a reason for hiding this comment

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

Hi @mattkrupnik. Thank you for your contribution. Could I ask you to extend the description slightly by adding information about what problem we are trying to solve, please? Also, you may add a note that the introduced approach exists for a non-current link.

Also, please, take a look at the failing B2B Unit tests. It looks like there's an assertion that needs to be adjusted.

Thank you!

Copy link
Contributor

@sidolov sidolov left a comment

Choose a reason for hiding this comment

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

@mattkrupnik please, sign Adobe CLA

@ghost ghost assigned sidolov Sep 16, 2020
@sidolov sidolov added Priority: P3 May be fixed according to the position in the backlog. Risk: medium Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. labels Sep 16, 2020
@mattkrupnik mattkrupnik requested a review from sidolov September 16, 2020 19:36
@mattkrupnik mattkrupnik changed the title Pass custom attributes to current link via XML [WIP]Pass custom attributes to current link via XML Sep 16, 2020
@mattkrupnik mattkrupnik changed the title [WIP]Pass custom attributes to current link via XML Pass custom attributes to current link via XML Sep 16, 2020
@mattkrupnik
Copy link
Contributor Author

@magento run Unit Tests

@mattkrupnik mattkrupnik requested a review from rogyar September 17, 2020 16:34
@rogyar
Copy link
Contributor

rogyar commented Sep 21, 2020

@magento run Unit Tests

@rogyar
Copy link
Contributor

rogyar commented Sep 22, 2020

@magento run all tests

@mattkrupnik
Copy link
Contributor Author

mattkrupnik commented May 6, 2021

@mattkrupnik please, sign Adobe CLA

@sidolov Can you approve? Your change request were realised.

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@mattkrupnik
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@mattkrupnik
Copy link
Contributor Author

@rogyar @sidolov Could you finish your review please?

@mattkrupnik mattkrupnik requested review from dudzio12, rogyar and sidolov and removed request for rogyar, sidolov and dudzio12 January 3, 2023 11:59
@mattkrupnik
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@engcom-Delta
Copy link
Contributor

@magento create issue

@engcom-Delta engcom-Delta added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Mar 18, 2024
@Priyakshic Priyakshic added the Project: Community Picked PRs upvoted by the community label Sep 12, 2024
@engcom-Dash
Copy link
Contributor

@magento run all tests

@engcom-Dash engcom-Dash self-assigned this Sep 13, 2024
@engcom-Dash
Copy link
Contributor

Hi @mattkrupnik ,

Thanks for the collaboration & contribution!

✔️ QA Passed
Preconditions:

Install fresh Magento 2.4-develop
Steps to reproduce
As per the Description.
Before: ✖️
Screenshot 2024-09-13 at 6 45 19 PM

After: ✔️
Screenshot 2024-09-13 at 6 41 56 PM

Builds are failed. Hence, moving this PR to Extended Testing.

Thanks

@engcom-Dash
Copy link
Contributor

@magento run all tests

@engcom-Dash
Copy link
Contributor

@magento run all tests

@engcom-Dash
Copy link
Contributor

engcom-Dash commented Sep 16, 2024

@engcom-Dash
Copy link
Contributor

@magento run Functional Tests CE, Functional Tests CE

@magento-devops-reposync-svc magento-devops-reposync-svc merged commit 4267628 into magento:2.4-develop Oct 18, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: View Priority: P3 May be fixed according to the position in the backlog. Progress: ready for testing Project: Community Picked PRs upvoted by the community Release Line: 2.4 Risk: medium Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue] Pass custom attributes to current link via XML
10 participants