Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Icons page #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Icons page #34

wants to merge 1 commit into from

Conversation

tonifisler
Copy link
Member

I have created a page with a table of all icons in the project!

We need to update the toolbox-utils task to generate the window variables:

window.icons = {
  "code": "Code",
  "search": "Search",
  "menu": "Menu",
  "toolbox": "Toolbox",
};

This example uses the icons from Toolbox ;)

@tonifisler tonifisler requested a review from Yago March 14, 2018 17:27
@Yago
Copy link
Member

Yago commented Mar 15, 2018

🤔 I don't know about adding something new to window. Usually, I do something between colors and a standard component. See the IMD component and the doc.

At least we can simply add the icons into data.json and use window.data. It's not a big deal to write a doc page to present the icons if there are some.

Also if we are using FontAwesome or any icon library out of our stack, we will still have this empty page... I think it's not as recurrent as the colors.

@Yago
Copy link
Member

Yago commented Mar 15, 2018

Maybe a lead is to add to the generator default styles and markup if they are choosing the SVG icon option. 🤔


return (
<div className="tlbx-icon-wrapper" key={key} title={key}>
<svg className="tlbx-icon"><use xlink="http://www.w3.org/1999/xlink" xlinkHref={`#icon-${key}`} /></svg>
Copy link
Member

Choose a reason for hiding this comment

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

We can use the Icon component to keep this logic in one place

@@ -16,6 +16,7 @@ import SingleFull from '../../views/Single/SingleFull';
import SinglePage from '../../views/Single/SinglePage';
import Doc from '../../views/Doc/Doc';
import Colors from '../../views/Colors/Colors';
import Icons from '../../views/Icons/Icons';
Copy link
Member

Choose a reason for hiding this comment

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

Icons will be a bit confusing with components/Icon/Icon and components/Icon/Icons...

return (
<div className="tlbx-grid">
{Object.keys(icons).map((key) => {
const icon = icons[key];
Copy link
Member

Choose a reason for hiding this comment

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

No need of a variable for so little and only one usage 😉

super();

this.state = {
icons: window.icons,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can add this logic in a proper Redux collection if it usefull somewhere else.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants