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

Remove dummy Say theming #263

Merged
merged 2 commits into from
May 9, 2022
Merged

Conversation

benjay10
Copy link
Contributor

Some theming (s-root) from the dummy app imported a file that would override colors and fonts used in the dummy test application and the plugins. This would interfere with the styling of the plugins, and caused uncertainty about the lay-out of the plugin (for me at least). With this file removed, the plugin would be rendered as if it were in the final editor, to be able to properly fine tune the lay-out.

@benjay10 benjay10 added the enhancement New feature or request label Apr 27, 2022
@benjay10 benjay10 requested review from Dietr and abeforgit April 27, 2022 14:45
@benjay10
Copy link
Contributor Author

I've realised something. We could keep this PR open for a bit longer and pile up more cleanup of the Say theming code, because we are working on porting all the plugins to (Ember-)AppUniversum.
Or we could make a new PR for more of that later.

@Dietr
Copy link
Contributor

Dietr commented Apr 29, 2022

We lost the global appuniversum variables with removing the custom variables so added these back by using the grouped appuniversum settings file.

I do wonder if using the appuniversum styling is correct here. We used to approach the editor as a standalone product (by styling it similar to https://say-editor.com/) which was not related to the style of Vlaanderen. This PR removes this custom style. It sure is easier to maintain this way. I don't know who decides on this styling? Maybe @nvdk

@nvdk
Copy link
Member

nvdk commented May 3, 2022

As long as we document how the styles can be customized I think we can drop the custom style for now.

@benjay10 benjay10 force-pushed the enhancement/remove-dummy-say-theming branch from 3d58bc7 to ace909d Compare May 9, 2022 15:52
@benjay10 benjay10 merged commit 23249ce into development May 9, 2022
@benjay10 benjay10 deleted the enhancement/remove-dummy-say-theming branch May 9, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants