-
Notifications
You must be signed in to change notification settings - Fork 21
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
Added data-popup attr to a tag #291
base: master
Are you sure you want to change the base?
Added data-popup attr to a tag #291
Conversation
Reviewer's Guide by SourceryThe PR adds functionality to copy the 'data-popup' attribute from parent anchor tags to burger menu items. This is implemented by retrieving the attribute value using jQuery's attr() method and conditionally setting it on the newly created anchor element if the attribute exists on the parent. Sequence diagram for adding 'data-popup' attribute to burger menu itemssequenceDiagram
participant User
participant ParentAnchor as Parent <a> Tag
participant BurgerMenu as Burger Menu Item
User->>ParentAnchor: Clicks on menu
activate ParentAnchor
ParentAnchor-->>BurgerMenu: Retrieve 'data-popup' attribute
deactivate ParentAnchor
activate BurgerMenu
BurgerMenu-->>BurgerMenu: Set 'data-popup' attribute if exists
deactivate BurgerMenu
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @vipulnarang95 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add a changelog entry following the conventional commits guidelines as mentioned in the checklist.
- Could you expand the PR description to explain why this data-popup attribute needs to be copied and what functionality it enables?
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
==========================================
- Coverage 84.19% 79.28% -4.92%
==========================================
Files 23 40 +17
Lines 1740 1868 +128
Branches 282 213 -69
==========================================
+ Hits 1465 1481 +16
- Misses 245 355 +110
- Partials 30 32 +2 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Description
Added an attribute from parent a tag to be included in burger menu item.
Attribute name: data-popup
Data popup is needed to open a link in burger menu in another window. this value is present in the original html of the link but not included while creating the burger menu.
for your reference: https://github.com/FidelityInternational/djangocms-content-expiry/blob/main/djangocms_content_expiry/templates/djangocms_content_expiry/calendar_icon.html
Related resources
Checklist
Summary by Sourcery
New Features: