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

Variables in svg attributes #302

Merged
merged 151 commits into from
Nov 16, 2014
Merged

Variables in svg attributes #302

merged 151 commits into from
Nov 16, 2014

Conversation

demos
Copy link
Contributor

@demos demos commented Jul 29, 2014

This PR allows skin developers to add variables in svg attributes.

Even if it's not finished, I wanted to show this work to get feedback before continuing.

For the moment, three use cases are available :

<tagname attr-name="value1 value3 var(varname)"/>

With it, "var(varname)" will be replace by the value of varname if it exists. If not, it will be replaced by an empty string (keep the vars at the end).

<tagname attr-name="prop1:val1; prop2:value2; var( prop3, varname );"/>

In this case, "var( prop3, varname );" will be replaced by "prop3:varvalue;" or removed if the variable doesn't exist in the skin context (variables have to be at the end of the attribute).

<tagname attr-name="default-value" var-attr-name="varname"/>

This syntax will trigger an overwriting of attr-name attribute by the value of varname if it exists.

The rules I followed to do it :

  • I chose a syntax looking like the "url()" css rules.
  • These syntaxes keep the svg file readable by a classical svg renderer (your web browser for example) but they have to be at the end of the attribute to do that.

I have adapted the skin I'm working on (https://github.com/demos/developer_skins/tree/master/Advanced-Player) to use variables in attributes and it allowed me to make it really simpler. Furthermore, this skin is now fully designed with svg so it should work perfectly on a retina screen.

The remaining questions :

  • Find a clever syntax for paths . Expressions look like good way to deal with it :
  • Support for encapsulated vars and expressions and chose a syntax: expr( 5 + var(height) ) or expr( 5 + height ).
  • I've temporary added a skincontext on the state elements of WPushButton to be able to change the colors or text of buttons depending on their state. Other usefull cases will probably appear in the future.
  • Globally, invent a coherent and user friendly syntax.

Thanks in advance.

@demos
Copy link
Contributor Author

demos commented Aug 3, 2014

As I didn't know which keywords to choose for calling variables in attributes, I added support for script elements in svg with a special object as property of the global one : templateHooks.

Each property of templateHooks is callable by it's key so the template designer can choose the keywords he needs depending of the use case.

Here is an example of a resizable slider handler : https://github.com/demos/developer_skins/blob/master/Advanced-Player/handle-vertical.svg

It looks like approaching a mergeable status but a review would be welcome and some cleanings with it, I think.

@jclaveau
Copy link

@rryan thanks a lot for the review (considering how much prs you review, it's not such a long delay :) )

If I'm right, all the comments are handled and the spinny widget and effetpushbutton support now svg.

But I think the naming is obsolete now : I make a PixmapSource from a filename which is used to make a "QImage" by wimagestore and "ImageSource"... (in the case of the spinny only as it uses QImage to spin fastly).

I wonder if I should merge ImageSource and PixmapSource but I need help to do that (too much noobyness sorry :3 )

@rryan
Copy link
Member

rryan commented Nov 16, 2014

Nice job @jclaveau! I have to say I don't fully understand the syntax you added to SVG yet (and that's partially because I don't know much about SVG) but all the other changes look solid and I'm sure this will come in super handy as we SVG-ize our release skins 👍.

@jclaveau
Copy link

@rryan Thank you! I really didn't expect to do that at the beginning.

Concerning the svg, the pattern applied to the attributes values of svg elements is configured in the svg script extension which allows the skin designer to customize it. I think it would be coherent to make the same with the pattern applied to the attributes names.

Waiting for the next review!

rryan added a commit that referenced this pull request Nov 16, 2014
Variables in svg attributes
@rryan rryan merged commit fab0ae2 into mixxxdj:master Nov 16, 2014
@jclaveau
Copy link

\o/

@ronso0 ronso0 mentioned this pull request Jan 28, 2017
@ferranpujolcamins
Copy link
Contributor

@demos @jclaveau Is this feature used? I think we need to remove it if we want to merge #1795 now.

@daschuer
Copy link
Member

I think this feature is currently unused, but why does it block #1795?

@ferranpujolcamins
Copy link
Contributor

Because somehow uses QScriptEngineDebugger, which is not yet ported to QJSEngine.

@jclaveau
Copy link

@ferranpujolcamins I don't think its used and I stopped spending time on it when I realized the limitations of Qt4 in terms stylesheets.

I personally used it on a theme which has never been enough complete due to it. Hopefully, one day, I would have some time to investigate making interfaces with Webkit and real js embedded.

Meanwhile, I was blocked by Qt CSS implementation (had to play with private API).

Does it answer your question?

@ferranpujolcamins
Copy link
Contributor

So is it ok for you if we remove this?

@Be-ing
Copy link
Contributor

Be-ing commented Nov 20, 2018

Yes, it was never documented, therefore no one knew about it and it has not been used.

@jclaveau
Copy link

jclaveau commented Nov 28, 2018

You can delete it :)

Hopefully one day Qt would have evolved concerning styling and scripting and I would have some time to make something less hacky

@ronso0
Copy link
Member

ronso0 commented Sep 28, 2019

@demos Would you mind sharing your working example?
Until now I didn't succed satisfactorily with a working implementation..

m0dB pushed a commit to m0dB/mixxx that referenced this pull request Jan 21, 2024
….15.0

Bump pygments from 2.11.2 to 2.15.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants