Skip to content

Conversation

@andersonjeccel
Copy link
Contributor

@andersonjeccel andersonjeccel commented Jul 10, 2024

Q A
Bug fix? (use the a.b branch) 🔴
New feature/enhancement? (use the a.x branch) 🟢
Deprecations? 🔴
BC breaks? (use the c.x branch) 🔴
Automated tests included? 🔴
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #13163

Description

Dependency:

This PR adds an accordion component. It's based on the Bootstrap JS logic (that's why there is a .panel in the code), but with unique style.

accordion.mp4

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. See if everything is the same across the interface, no changes since it's a technical addition

But PRs where you can interact with it:

@andersonjeccel andersonjeccel requested review from a team, Mike-Dropsolid and ricfreire July 10, 2024 11:10
@andersonjeccel andersonjeccel self-assigned this Jul 10, 2024
@andersonjeccel andersonjeccel added T1 Low difficulty to fix (issue) or test (PR) user-interface Anything related to appearance, layout, and interactivity ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality developer-experience Anything related to developer experience labels Jul 10, 2024
@andersonjeccel andersonjeccel added this to the 5.2 milestone Jul 10, 2024
@codecov
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.51%. Comparing base (95eff6a) to head (f856fae).
Report is 1 commits behind head on 5.x.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                5.x   #13940   +/-   ##
=========================================
  Coverage     62.51%   62.51%           
  Complexity    34353    34353           
=========================================
  Files          2260     2260           
  Lines        102740   102740           
=========================================
  Hits          64233    64233           
  Misses        38507    38507           

This was referenced Jul 10, 2024
@escopecz
Copy link
Member

@andersonjeccel here are some conflicts

@andersonjeccel
Copy link
Contributor Author

@escopecz Solved!

@escopecz escopecz added the blocked Something blocks this PR/issue (e.g. waiting for another PR to be merged) label Jul 19, 2024
Copy link
Contributor

@LordRembo LordRembo left a comment

Choose a reason for hiding this comment

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

Some comments for the code added

@LordRembo LordRembo added code-review-passed PRs which have passed code review and removed code-review-needed PR's that require a code review before merging labels Aug 1, 2024
Copy link
Contributor

@LordRembo LordRembo left a comment

Choose a reason for hiding this comment

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

Code-wise, seems okay now. Works as intended too.

@LordRembo
Copy link
Contributor

LordRembo commented Aug 1, 2024

The description of the PR could use a link to a page of where to test it properly (path or url). That's not immediately clear from the video.

@andersonjeccel
Copy link
Contributor Author

Copy link
Member

@abhisekmazumdar abhisekmazumdar left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@andersonjeccel andersonjeccel added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test blocked Something blocks this PR/issue (e.g. waiting for another PR to be merged) labels Aug 9, 2024
@andersonjeccel andersonjeccel added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged user-testing-passed PRs which have been successfully tested by the required number of people. and removed pending-test-confirmation PR's that require one test before they can be merged labels Aug 14, 2024
@andersonjeccel
Copy link
Contributor Author

Warning

All Tier 1 PRs need only 1 code review and 1 user testing before able to merge. I'll update accordingly.

@RCheesley
Copy link
Member

Just waiting for the tests to pass then I'll get this one merged! 🚀

@RCheesley RCheesley merged commit c5f5095 into mautic:5.x Aug 20, 2024
@andersonjeccel andersonjeccel deleted the ui-accordions branch August 20, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-review-passed PRs which have passed code review developer-experience Anything related to developer experience enhancement Any improvement to an existing feature or functionality ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged T1 Low difficulty to fix (issue) or test (PR) user-interface Anything related to appearance, layout, and interactivity user-testing-passed PRs which have been successfully tested by the required number of people.

Projects

Archived in project
Status: Done

Development

Successfully merging this pull request may close these issues.

Align accordion to the design system

5 participants