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

Fed Icons - Table icon decoration and collapse icon wrapping #3202

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

Sartxi
Copy link
Contributor

@Sartxi Sartxi commented Nov 14, 2024

In reference to the new federal icons work that was reverted, icons within tables were not having their respective SVG assets injected or tooltips being decorated. This was because the table block moves the icons around. I was able to see the icons being decorated by reselecting all icons before passing to icon js to be decorated and loaded.

There is also an issue where the expand/collapse icons wrapped to the bottom because of the new node index margin that was added. Unsetting that for this individual icon in the table.css.

Resolves: MWPW-140452

Test URLs:

URLs where issue was uncovered:

@Sartxi Sartxi added the bug Something isn't working label Nov 14, 2024
Copy link
Contributor

aem-code-sync bot commented Nov 14, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link
Contributor

aem-code-sync bot commented Nov 14, 2024

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (rparrish/fed-icons@d97afa4). Learn more about missing BASE report.

Additional details and impacted files
@@                  Coverage Diff                  @@
##             rparrish/fed-icons    #3202   +/-   ##
=====================================================
  Coverage                      ?   96.38%           
=====================================================
  Files                         ?      245           
  Lines                         ?    56696           
  Branches                      ?        0           
=====================================================
  Hits                          ?    54646           
  Misses                        ?     2050           
  Partials                      ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@Sartxi Sartxi changed the title Fed Icons - icons not being decorated in Tables Fed Icons - Table icon decoration and collapse icon wrapping Nov 15, 2024
Copy link
Contributor

@ryanmparrish ryanmparrish 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. Heads up - the before/after URLs in the description have a malformed url param: ? instead of & on the second param martech=off.

@ryanmparrish ryanmparrish merged commit 77b2a12 into rparrish/fed-icons Nov 25, 2024
13 checks passed
@ryanmparrish ryanmparrish deleted the sartxi/fed-icons-tables branch November 25, 2024 16:36
overmyheadandbody pushed a commit that referenced this pull request Dec 3, 2024
* reselect icons after area is decorated

* fix inline margin issue in table collapse titles
milo-pr-merge bot pushed a commit that referenced this pull request Dec 9, 2024
…idual SVG assets (#3259)

* updated features/icons to pull from federal and allow the icons set to be sharepoint authorable

* bettter icon-spacing handling

* preload federated.js instead of non used icons.svg on util decorateIcon()

* Updated preload federated as script type not fetch

* Minor clean up based on pr feedback

* remove console

* addressed some issues found w/ icons not rending in tables due to race condition w/ decorateIcon()

* lana instead of console

* no 100% height

* coverage

* full coverage

* minor cleanup

* minor cleanup

* preload first section icons, spread syntax

* no cons

* Performace refactor - loadIcons to hold most related functionallity. Preloaded inView icons and defered others till postSectionLoad.

* async decTooltips and refactor to not chain func calls

* fix: icon alignment in georouting modal's button

* move render blocking code to utils

* fix import of test method

* remove condition that causes error when no icon is in view

* remove redundant link load

* remove extra federated.js import

* small change to push branch

* Fed Icons - Table icon decoration and collapse icon wrapping (#3202)

* reselect icons after area is decorated

* fix inline margin issue in table collapse titles

---------

Co-authored-by: Saugat Malla <saugat@TBXTOR-2021MBP16-SaugatMalla.local>
Co-authored-by: Saugat Malla <saugat013@gmail.com>
Co-authored-by: Sartxi <sean.archibeque@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants