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

Dashicons use wp.element.Component instead of importing it #3952

Closed
jaschaio opened this issue Dec 12, 2017 · 15 comments
Closed

Dashicons use wp.element.Component instead of importing it #3952

jaschaio opened this issue Dec 12, 2017 · 15 comments
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Code Quality Issues or PRs that relate to code quality

Comments

@jaschaio
Copy link

jaschaio commented Dec 12, 2017

I noticed the Following inconsistency which gives me errors if I try to reutilize certain components in a different context than the WordPress Admin. All other components within the components directory import { Component } from '@wordpress/element';. But the Dashicons directly extends wp.element.Component instead of importing and using the { Component } class from @wordpress/element.

That makes almost any component that imports the Dashicons depending on that the wp.element object is defined. Maybe I am overlooking something here but I felt that its an inconsistency and couldn't immediately understand why.

Please let me know as well if this issue should be raised in the Dashicon repro instead as apparently that's where the file is coming from.

@danielbachhuber danielbachhuber added the [Status] Needs More Info Follow-up required in order to be actionable. label Jan 23, 2018
@danielbachhuber
Copy link
Member

Hi @jaschaio,

Thanks for the report. How'd you end up on this? Did you try git blame to track down the source of each implementation?

@gziolo
Copy link
Member

gziolo commented Feb 1, 2018

@jasmussen should know the details. I think that Dashicons are automatically generated outside of Gutenberg and that's why they reference wp.element.Component.

@jasmussen
Copy link
Contributor

Not sure if this answers your questions, but Dashicons are built from this repo: https://github.com/WordPress/dashicons — you clone that, npm install and npm run build. That then converts a folder of SVGs into a React component.

We can adjust what that React component contains, though, so if changes need to happen in how that component is built, we can fix it upstream.

@gziolo
Copy link
Member

gziolo commented Feb 1, 2018

It might be tricky to fix it at the moment because we don't publish @wordpress/element to npm. We should get back to it later when we start merging this code into core.

@danielbachhuber
Copy link
Member

@gziolo Do you know if this was addressed, or needs further action?

@gziolo
Copy link
Member

gziolo commented May 21, 2018

We should publish the first alpha version of @wordpress/element later this week. It should unblock this issue.

@gziolo gziolo added [Type] Code Quality Issues or PRs that relate to code quality [Status] Blocked Used to indicate that a current effort isn't able to move forward and removed [Status] Needs More Info Follow-up required in order to be actionable. labels May 21, 2018
@gziolo
Copy link
Member

gziolo commented May 21, 2018

Depends on #6708.

@gziolo gziolo added this to the 3.0 milestone May 21, 2018
@youknowriad youknowriad modified the milestones: 3.0, 3.1 May 29, 2018
@gziolo gziolo removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Jun 11, 2018
@gziolo
Copy link
Member

gziolo commented Jun 11, 2018

We can use @wordpress/element published to NPM: https://www.npmjs.com/package/@wordpress/element.

Related code line to update: https://github.com/WordPress/gutenberg/blob/master/components/dashicon/index.js#L16.

@jasmussen does it require any external changes, too?

@gziolo gziolo added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Jun 11, 2018
@gziolo gziolo removed this from the 3.1 milestone Jun 19, 2018
@jasmussen
Copy link
Contributor

Sorry I missed the ping.

I'm not sure what change needs to be made, but if a change to the dashicons component needs to happen I can do that change quickly. Just let me know what to change that line to.

@gziolo
Copy link
Member

gziolo commented Jun 28, 2018

import { Component } from '@wordpress/element'; - you need to use this import statement instead oglf WP global.

@jasmussen
Copy link
Contributor

@gziolo Apologies for being slow. I think it's the following file that needs to be updated: https://github.com/WordPress/dashicons/blob/master/sources/react/index-header.jsx#L16 — this is a source file that is being used to build the React component with a grunt script. I can add the import, but I still need an export, no?

@gziolo
Copy link
Member

gziolo commented Jun 28, 2018

I think that import is fine. I think @aduth should know better what is the best way to tackle it. I have no background knowledge of when and how this repository is used. You might need to set @wordpress/element as a dependency for the repository. Do you copy output file each time you want to regenerate it?

@jasmussen
Copy link
Contributor

Right now the process for creating a new dashicon is:

  • Save new SVG icon file in sources/gutenberg — when put in that directory it lands ONLY in the react component. This was done to avoid having to get design approval on a per-icon basis, and instead get design approval for all of them at once, come merge-time.
  • npm run build in the dashicons repo puts together the react component header file, a for loop with output of each of the icons inline, then the react component footer file.
  • This process bakes a .js file which I then manually copy to the Gutenberg project, where the file is incidentally a jsx file.

It doesn't have to be this way — I'm just confused as to how to proceed.

@gziolo
Copy link
Member

gziolo commented Jun 28, 2018

I'm confused, too. Maybe @mtias can help, too?

@aduth
Copy link
Member

aduth commented Jun 28, 2018

See WordPress/dashicons#305

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

6 participants