Implement StyleEditor (encompasses a Color Picker) #3329

FranciscoTGouveia · 2024-05-28T22:13:53Z

Instead of using the StyleSheet, one can now use the StyleEditor. The StyleEditor is a GUI that allows to style elements by their name and is coherent with the StyleSheet.

PR Checklist

Please check if your PR fulfills the following requirements:

I have read, and I am following the Contributor guide
I have read, and I understand the GNOME Code of Conduct

PR Type

What kind of change does this PR introduce?

Bug fix
Feature
Chore (refactoring, formatting, local variables, other cleanup)
Documentation content changes

What is the current behavior?

Right now, when we want to change the style of an Element we would need to go on the StyleSheet and reference it by it's name.
Many people aren't familiar with CSS and this can stand in their way while developing their diagrams.

Issue Number: #2136

What is the new behavior?

Instead of using the StyleSheet, one can now use the StyleEditor, a GUI that opens up from a button in the property pages. This allows a user to style their elements with visual cues and abstract themselves from the details of CSS and how it works under the hood.

Does this PR introduce a breaking change?

Yes
No

Working example

Screencast.from.2024-05-28.23-10-07.webm

Important notes

When you style an element, you are styling that specific element (by their name) and not all elements of that type. This is true for all named elements except relationships.
The changes made in the GUI (StyleEditor) are coherent with the StyleSheet.
The changes made in the StyleEditor are only exported to the StyleSheet when asked, or when the file is saved.
The properties that the StyleEditor offers are related to "colors" (in order to fix the issue), and "border-radius", but more properties can be added (hopefully in a near future).

Implementation details to discuss

We thought of a whole interface to abstract the CSS. Is this a good idea (that can be expanded), or shall we stay with just a simple Color Picker ?
The changes that we made with the StyleEditor are only being written to the StyleSheet upon request or when saving the file. Is this a good implementation or should we update the StyleSheet everytime we make a change on the StyleEditor ?
Since we didn't find a class that would cover all of the Presentation representations with names (i.g:. some inherit from Named, others from ElementPresentation, etc) we ended up using the most general one, Presentation. However, this means that all the relationships wouldn't be able to be referenced by name. Moreover, we had to add a try-except clause on the load method of Presentation since some Items don't have names.

Authors:
Francisco Gouveia: francisco.t.gouveia@tecnico.ulisboa.pt
João Correia: joao.marques.correia@tecnico.ulisboa.pt

Instead of using the StyleSheet, one can now use the StyleEditor. The StyleEditor is a GUI that allows to style elements by their name and is coherent with the StyleSheet. Fixes gaphor#2136 Co-authored-by: Francisco Gouveia <francisco.t.gouveia@tecnico.ulisboa.pt>

amolenaar · 2024-05-29T21:52:24Z

This looks pretty good.

We thought of a whole interface to abstract the CSS. Is this a good idea (that can be expanded), or shall we stay with just a simple Color Picker ?

I think it's a good idea to keep the option for expansion open at least.

The changes that we made with the StyleEditor are only being written to the StyleSheet upon request or when saving the file. Is this a good implementation or should we update the StyleSheet everytime we make a change on the StyleEditor ?

Is it easy to update elements in the. style sheet directly? It would be my preference since all styling is then in one place, but I can imagine it's requires a lot of processing to update.

Since we didn't find a class that would cover all of the Presentation representations with names (i.g:. some inherit from Named, others from ElementPresentation, etc) we ended up using the most general one, Presentation. However, this means that all the relationships wouldn't be able to be referenced by name. Moreover, we had to add a try-except clause on the load method of Presentation since some Items don't have names.

I think you can just add one to Presentation and check if the subject exists and has a name attribute.
IIRC it becomes simpler once we start using KerML as our core model, since Element will have a name.

A few suggestions: I would move the StyleEditor to gaphor.ui, since it's a (g)ui related feature. I would also avoid abbreviated terms, especially in public methods (style_elements rather than style_elems).

0lione · 2024-06-03T11:08:58Z

This looks pretty good.

We thought of a whole interface to abstract the CSS. Is this a good idea (that can be expanded), or shall we stay with just a simple Color Picker ?

I think it's a good idea to keep the option for expansion open at least.

The changes that we made with the StyleEditor are only being written to the StyleSheet upon request or when saving the file. Is this a good implementation or should we update the StyleSheet everytime we make a change on the StyleEditor ?

Is it easy to update elements in the. style sheet directly? It would be my preference since all styling is then in one place, but I can imagine it's requires a lot of processing to update.

Since we didn't find a class that would cover all of the Presentation representations with names (i.g:. some inherit from Named, others from ElementPresentation, etc) we ended up using the most general one, Presentation. However, this means that all the relationships wouldn't be able to be referenced by name. Moreover, we had to add a try-except clause on the load method of Presentation since some Items don't have names.

I think you can just add one to Presentation and check if the subject exists and has a name attribute. IIRC it becomes simpler once we start using KerML as our core model, since Element will have a name.

A few suggestions: I would move the StyleEditor to gaphor.ui, since it's a (g)ui related feature. I would also avoid abbreviated terms, especially in public methods (style_elements rather than style_elems).

I feel the same way, however with our approach when an element changes it's name or when it's removed or even when it's loaded from a file, our "backend" takes care of this and updates the element accordingly. If we were to simply write the changes into the StyleSheet then, whenever the user deleted an element or change it's name there would be useless lines in the StyleSheet. For the backend to track these changes in the StyleSheet would be computationally heavy. That's why we've created the export button, to allow user to look under the hood if he wants to.

Co-authored-by: Francisco Gouveia <francisco.t.gouveia@tecnico.ulisboa.pt>

…agram Co-authored-by: João Correia <joao.marques.correia@tecnico.ulisboa.pt>

0lione · 2024-06-08T11:14:25Z

Hi @amolenaar , I think we've already addressed the requested changes. Could we request another review?

amolenaar

Thanks @0lione, sorry for the late feedback.

Looks good. I think for now it's good to be specific on which elements have a custom styling.

I proposed some (mainly stylistic) changes. I have not been able to run the code yet.

gaphor/UML/classes/datatype.py

gaphor/UML/deployments/node.py

gaphor/core/modeling/presentation.py

gaphor/core/modeling/stylesheet.py

tests/test_architecture.py

Co-authored-by: João Correia <joao.marques.correia@tecnico.ulisboa.pt>

Co-authored-by: Francisco Gouveia <francisco.t.gouveia@tecnico.ulisboa.pt>

danyeaw · 2024-10-26T19:46:12Z

Hi @FranciscoTGouveia, what is remaining to get this merged?

FranciscoTGouveia · 2024-10-28T10:52:58Z

Hello @danyeaw! I think it is ready to be merged. In my understanding this could be a good substitute for the Style Sheet as it is, offering a graphical and user-friendly way of styling elements. However, this still has room for improvement (e.g. not all style options are available). With that said, I think that feedback from the community would be beneficial to know what path to follow regarding the Style Editor.

danyeaw · 2024-10-29T01:05:32Z

Thanks @FranciscoTGouveia, would you mind please resolving the merge conflicts?

FranciscoTGouveia · 2024-11-10T23:00:45Z

Hi @danyeaw ! While resolving the conflicts I found out that the button for opening the Style Editor is not showing. As far as I can tell, this is related with the fact that the StylePropertyPage is now defined in a separate file from the other property pages.

To fix this, I could just import the diagram/styleeditor inside of diagram/propertypages but this would cause a circular import. While doing the import in the end of the file works, it doesn't seem like a good practice. With that said, should I undo the last commit and leave the StylePropertyPage in the diagram/propertypages or am I missing something?

CC @amolenaar

amolenaar · 2024-11-29T11:55:32Z

I was wondering if the Open Style Editor button should be placed below or above the CSS editor area. Since styles are applied via the style sheet, it would make it clear to users where the styles go. The editor can be used as a tool to simply define the style.

I think we can move some logic up to the Presentation class, so that it does not have to be defined for every model element.

Style editor now works on one element, the focused one. The style is updated instantly, but it needs to be added to the style sheet in order to be persisted.

Reverting unneeded changes

amolenaar · 2024-11-30T16:29:35Z

I simplified the code a bit. Instead of having styling on every element, the style editor adds its (temporary) styling to StyleSheet.instant_style_declarations. To ensure diagrams are updated, I added a new event type: StyleSheetUpdated.

The colors are updated based on the current style of the selected item.
The styling is only persisted if you add it to the style sheet.

I updated the layout of the style editor:

danyeaw

This looks great, thanks so much @FranciscoTGouveia and @0lione! Thanks @amolenaar for the final update - integrating it with the style editor like this is really good idea as well.

danyeaw · 2025-01-20T20:08:16Z

@all-contributors please add @FranciscoTGouveia for code

allcontributors · 2025-01-20T20:08:19Z

@danyeaw

@FranciscoTGouveia already contributed before to code

danyeaw · 2025-01-20T20:08:41Z

@all-contributors please add @0lione for code

allcontributors · 2025-01-20T20:08:44Z

@danyeaw

@0lione already contributed before to code

github-actions bot added the python Pull requests that update Python code label May 28, 2024

0lione and others added 2 commits June 3, 2024 13:06

fix(color_picker): Remove try block and abbreviated terms

c1c3cf6

Co-authored-by: Francisco Gouveia <francisco.t.gouveia@tecnico.ulisboa.pt>

fix(color_picker): Move StyleEditor to gaphor.ui instead of gaphor.di…

e89c9a4

…agram Co-authored-by: João Correia <joao.marques.correia@tecnico.ulisboa.pt>

amolenaar reviewed Jun 18, 2024

View reviewed changes

FranciscoTGouveia and others added 3 commits June 19, 2024 09:47

fix(color_picker): Delete rule on test_architecture

143be10

Co-authored-by: João Correia <joao.marques.correia@tecnico.ulisboa.pt>

fix(color_picker): Stylistic changes

8c55efa

Co-authored-by: Francisco Gouveia <francisco.t.gouveia@tecnico.ulisboa.pt>

Move all style editor code to one place.

d1d440f

amolenaar mentioned this pull request Jul 10, 2024

Control panel for CSS #3380

Closed

FranciscoTGouveia requested a review from amolenaar November 21, 2024 18:23

amolenaar added 6 commits November 29, 2024 21:24

Move PresentationStyle to one place

472bb40

What's this weird thing with imports?

70ec95f

Cleanup

4b0f624

Merge branch 'main' into style-editor-feature

1b0ba11

Simplify style editor

7d7c396

Style editor now works on one element, the focused one. The style is updated instantly, but it needs to be added to the style sheet in order to be persisted.

Boyscouting

d6a3066

Reverting unneeded changes

danyeaw approved these changes Nov 30, 2024

View reviewed changes

danyeaw added feature A new feature and removed python Pull requests that update Python code labels Nov 30, 2024

danyeaw merged commit 4202aef into gaphor:main Nov 30, 2024
23 checks passed

FranciscoTGouveia deleted the style-editor-feature branch December 1, 2024 10:08

FranciscoTGouveia restored the style-editor-feature branch December 1, 2024 10:08

Uh oh!

Implement StyleEditor (encompasses a Color Picker) #3329

Implement StyleEditor (encompasses a Color Picker) #3329

Uh oh!

Conversation

FranciscoTGouveia commented May 28, 2024

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Working example

Important notes

Implementation details to discuss

Uh oh!

amolenaar commented May 29, 2024

Uh oh!

0lione commented Jun 3, 2024

Uh oh!

0lione commented Jun 8, 2024

Uh oh!

amolenaar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danyeaw commented Oct 26, 2024

Uh oh!

FranciscoTGouveia commented Oct 28, 2024

Uh oh!

danyeaw commented Oct 29, 2024

Uh oh!

FranciscoTGouveia commented Nov 10, 2024

Uh oh!

amolenaar commented Nov 29, 2024

Uh oh!

amolenaar commented Nov 30, 2024

Uh oh!

danyeaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danyeaw commented Jan 20, 2025

Uh oh!

allcontributors bot commented Jan 20, 2025

Uh oh!

danyeaw commented Jan 20, 2025

Uh oh!

allcontributors bot commented Jan 20, 2025

Uh oh!

Uh oh!