jarek-foksa / xel

Xel - Widget toolkit for building native-like Electron and Web apps
https://xel-toolkit.org
Other
682 stars 59 forks source link

icons are breaking #25

Closed sandor closed 7 years ago

sandor commented 7 years ago

using this will show the icons as intended (see first screenshot):

                    <div class="Grid-cell Grid--full">
                        <div class="content">
                            <x-buttons style="padding:4px;" tracking="1">
                                <x-button skin="plain">
                                    <x-icon name="edit"></x-icon>
                                    <x-label style="margin-left:4px;">Editor</x-label>
                                </x-button>

                                <x-button skin="plain">
                                    <x-icon name="visibility"></x-icon>
                                    <x-label style="margin-left:4px;">Preview</x-label>
                                </x-button>

                                <x-button skin="plain">
                                    <x-icon name="code"></x-icon>
                                    <x-label style="margin-left:4px;">Code</x-label>
                                </x-button>
                            </x-buttons>
                        </div>
                    </div>
                    <div class="Grid-cell right-menu">
                        <div class="content">
                            <x-buttons tracking="1">
                                <x-button skin="iconic">
                                    <x-box vertical>
                                        <x-icon name="extensions-color"></x-icon>
                                        <x-label>Editor</x-label>
                                    </x-box>
                                </x-button>

                                <x-button skin="iconic">
                                    <x-box vertical>
                                        <x-icon name="visibility"></x-icon>
                                        <x-label>Preview</x-label>
                                    </x-box>
                                </x-button>

                            </x-buttons>
                        </div>
                    </div>

changing the icon name (extension-color) to edit will break the icons (screenshot2):

                    <div class="Grid-cell Grid--full">
                        <div class="content">
                            <x-buttons style="padding:4px;" tracking="1">
                                <x-button skin="plain">
                                    <x-icon name="edit"></x-icon>
                                    <x-label style="margin-left:4px;">Editor</x-label>
                                </x-button>

                                <x-button skin="plain">
                                    <x-icon name="visibility"></x-icon>
                                    <x-label style="margin-left:4px;">Preview</x-label>
                                </x-button>

                                <x-button skin="plain">
                                    <x-icon name="code"></x-icon>
                                    <x-label style="margin-left:4px;">Code</x-label>
                                </x-button>
                            </x-buttons>
                        </div>
                    </div>
                    <div class="Grid-cell right-menu">
                        <div class="content">
                            <x-buttons tracking="1">
                                <x-button skin="iconic">
                                    <x-box vertical>
                                        <x-icon name="edit"></x-icon>
                                        <x-label>Editor</x-label>
                                    </x-box>
                                </x-button>

                                <x-button skin="iconic">
                                    <x-box vertical>
                                        <x-icon name="visibility"></x-icon>
                                        <x-label>Preview</x-label>
                                    </x-box>
                                </x-button>

                            </x-buttons>
                        </div>
                    </div>

1 2

jarek-foksa commented 7 years ago

Can you reproduce this issue with Electron 1.7.x? I suspect this is caused by Chrome bug 597080 which was still present in older Electron versions.

sandor commented 7 years ago

sorry for my ignorance jarek – I am using the prebuilt version 1.6.8 (I think last stable?) and I don't know how to update to 1.7.x ... (a quick tip here for a newbie?) But anyhow this seems to be the problem. If I build a new iconset like the one you are using (the boxy-svg stuff) and I use ONLY those icons than the icons seems to work well...

sandor commented 7 years ago

hmmm... made some further testing. I suppose that there is somehow a conflict if we use the "Iconic" skin and other icons than the your icons from icons.svg...

jarek-foksa commented 7 years ago

You can download Electron 1.7.1 from https://github.com/electron/electron/releases, it's still in beta stage, but I haven't noticed any issues so far.

jarek-foksa commented 7 years ago

If I recall correctly, the old Chrome bug was triggered when there were two or more icons with the same name (like <x-icon name="edit"></x-icon> in your code).

sandor commented 7 years ago

thanks! using 1.7.1 solves the problem with the icons...

sandor commented 7 years ago

sorry, unrelated question: seeing my screenshots above – how would I align the first button group left the second group in the middle and the third group on the right of my header using your x-box ?

jarek-foksa commented 7 years ago

It's hard to say by looking at the code you pasted as you seem to be using some grid system. I would try applying justify-content: center; and align-items: center; on the <x-box> and/or other containers.

sandor commented 7 years ago

awesome! thanks... justify-content: space-between; was the answer :-)

jarek-foksa commented 7 years ago

I have updated the minimal Electron version required by Xel in the docs, so we can close this now.