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

Spike: investigate how we are importing & overriding styles from @wordpress/components #8012

Open
mgascam opened this issue Jan 12, 2024 · 9 comments
Labels
category: core WC Payments core related issues, where it’s obvious. focus: misc or unknown Issues that need to be added to a focus area (aka "needs focus"). priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability type: spike

Comments

@mgascam
Copy link
Contributor

mgascam commented Jan 12, 2024

Refocusing this as a spike to better understand the problem. Spike will need to answer these questions:

  1. How are we importing @wordpress/components.
    • Are we on a recent version, i.e. should we update.
    • Do we import from npm i.e. bundle, or do we depend on core (or do we do both!)
  2. What is the recommended way to use @wordpress/components and get the styles.
    • It's not clear what the correct approach is. I would expect that we should be able to get styles "for free" if we bundle, or if we use the version built in to WordPress core.
  3. What styles we are overriding (from @wordpress/components).
    • Based on info below, it seems we are overriding (customising) some styles, and this is causing problems.
    • This should include a mini-audit to understand the extent – i.e. do we have 1-5 minor tweaks or are we overriding lots of components (and then overriding those overrides internally… 😁 )

Once we know the options for reusing styles from the package (1 and 2 above), then we can make a plan for how to iterate towards reducing or removing our customisations/overrides (3).

Read on for more info in original bug report (i.e. a symptom caused by current situation).

Describe the bug

There are missing CSS rules from the wordpress/components package in the compiled stylesheet for WC Admin that cause visual issues.

For context, we noticed this issue while working on #7742. One of the requirements was to introduce a dropdown menu on the payment details screen. We decided to use the DropdownMenu from the wordpress/components package, but noticed that the positioning was not working. After investigation, it seemed that there were missing rule definitions in the CSS stylesheet of the WP Admin related to the wordpress/components.

In my Docker based dev environment, these styles are loaded in http://localhost:8082/wp-includes/css/dist/components/style.css?ver=6.4.2

To fix the missing styles we started importing the missing styles from the node_modules in the React component (here), but we needed to revert since this was causing visual issues in other instances of DropdownMenu.

To Reproduce

N.B: This has been spotted in the payment details page with the DropdownMenu component, but potentially affects other parts of the codebase where wordpress/components are utilized.

  1. In your local WooPayments instance, remove the CSS workaround here and rebuild CSS (i.e. npm start)
  2. Navigate to Transactions > Payment Details page
  3. Click on the three-dots menu on the top right corner of the payment summary.

Actual behavior

The DropdownMenu is misplaced in the layout:

Screenshot 2024-01-16 at 14 06 36

Screenshots

Expected behavior

The DropdownMenu is properly placed in the layout:

Screenshot 2024-01-16 at 14 08 40

Additional context

Slack discussion: https://a8c.slack.com/archives/C053VQPD74J/p1700490601899939

@mgascam mgascam added the type: bug The issue is a confirmed bug. label Jan 12, 2024
@jessy-p
Copy link
Contributor

jessy-p commented Jan 15, 2024

@mgascam Could you please add the details under TODO so that the issue can be assigned to the responsible team.

@mgascam
Copy link
Contributor Author

mgascam commented Jan 16, 2024

@mgascam Could you please add the details under TODO so that the issue can be assigned to the responsible team.

Thanks for the ping @jessy-p, I added more context. Hopefully it's more clear now, but It's kind of a tricky issue so please let me know if you have further questions.

@kalessil kalessil added the component: streamline refunds Issues related to Streamline refunds project label Jan 18, 2024
@zmaglica zmaglica added the category: core WC Payments core related issues, where it’s obvious. label Jan 25, 2024
@kalessil kalessil added type: developer experience and removed component: streamline refunds Issues related to Streamline refunds project labels Feb 5, 2024
@kalessil
Copy link
Contributor

kalessil commented Feb 5, 2024

This should be a coordinated effort here, as this is related to the JS components version used in the development and provided by the runtime.

EDIT: removing from Pulsars board, this issue is for our JS guild lo look into.

@haszari haszari added the focus: misc or unknown Issues that need to be added to a focus area (aka "needs focus"). label Mar 11, 2024
@vbelolapotkov
Copy link
Collaborator

@haszari sending this one to your team's maintenance backlog. I've assigned the issue to you for this, please take care of adding it to the team board.

@haszari
Copy link
Contributor

haszari commented Dec 15, 2024

Thanks @vbelolapotkov – added to our board, will set a priority soon.

@haszari haszari added the needs prioritisation Triage finished and issues are ready for the following processing. label Dec 17, 2024
@haszari
Copy link
Contributor

haszari commented Dec 17, 2024

I will take another look at this to better understand the problem so helix can estimate and action. @haszari

@haszari
Copy link
Contributor

haszari commented Dec 17, 2024

A good next step here is a spike to understand:

  1. How are we importing @wordpress/components.
    • Are we on a recent version, i.e. should we update.
    • Do we import from npm i.e. bundle, or do we depend on core (or do we do both!)
  2. What is the recommended way to use @wordpress/components and get the styles.
    • It's not clear from issue description what the correct approach is. I would expect that we should be able to get styles "for free" if we bundle, or if we use the version built in to WordPress core (or potentially via WooCommerce core??).
  3. What styles we are overriding (from @wordpress/components).
    • It's clear from the description that we are overriding (customising) some styles, and this is causing problems.
    • This should include a mini-audit to understand the extent – i.e. do we have 1-5 minor tweaks or are we overriding lots of components (and then overriding those overrides internally… 😁 )

Once we know the options for reusing styles from the package (1 and 2 above), then we can make a plan for how to iterate towards reducing or removing our customisations/overrides (3).

I'll update the title and description to focus on this spike, we can log other issues once we have more info! FYI @mgascam let me know if I'm missing anything, any feedback you have on this plan of action.

@haszari haszari added type: spike and removed type: bug The issue is a confirmed bug. labels Dec 17, 2024
@haszari haszari changed the title Missing CSS rules from wordpress/components in the compiled CSS stylesheet Spike: investigate how we are importing & overriding styles from @wordpress/components Dec 17, 2024
@haszari haszari removed their assignment Dec 17, 2024
@haszari haszari added the priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability label Dec 17, 2024
@haszari
Copy link
Contributor

haszari commented Dec 17, 2024

Prioritised as medium, since it could have a positive impact – i.e. speed up development if we just depend on components, reduced or no overrides.

That said, there's no urgency here, this could be low.

@haszari haszari removed the needs prioritisation Triage finished and issues are ready for the following processing. label Dec 17, 2024
@Jinksi
Copy link
Member

Jinksi commented Dec 20, 2024

Are we on a recent version, i.e. should we update.

A quick answer to this, as I was investigating for a discussion in #8382

WooPayments is importing @wordpress/components 19.8.5, published 2 years ago.

The current version of @wordpress/components is 29.0.0.

Unfortunately, WooPayments has not kept up-to-date with component library versions as part of regular maintenance, and so an update to the current version will require effort that has not been easy to prioritise in the past – @Automattic/helix talked about this at our last meetup pdjTHR-3SB-p2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: core WC Payments core related issues, where it’s obvious. focus: misc or unknown Issues that need to be added to a focus area (aka "needs focus"). priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability type: spike
Projects
None yet
Development

No branches or pull requests

9 participants