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

chore: enable live reload for changes to themes #5668

Conversation

rpocklin
Copy link
Contributor

@rpocklin rpocklin commented Aug 13, 2021

Description

Owncloud has done a great job allowing customisations of themes without needing to make modifications directly to the owncloud-design-system 👍

This PR allows you to live reload in local development mode as you go through and change any theme(s) you are working on. (Eg. changing the colour of swatch-brand-default.)

My understanding for why it does not work by default is because the application will load the theme file via fetch at runtime from the location defined in the config.json which, typically, would be defined as follows (on the same domain as a relative url):

   "theme": "themes/aarnet/theme.json",

This content is generated from packages/web-runtime/themes/<theme_name>/theme.json and copied by rollup into the dist folder, but because it's not part of the source code of the project (retrieved via fetch as opposed to static includes via TS/JS) rollup does not monitor changes to files in these folders.

There may be a better way to do this so open to alternatives but this is an easy way to get the intended behaviour of real-time theme customisations, which makes it a very productive way to customise the look and feel.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

@update-docs
Copy link

update-docs bot commented Aug 13, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@sonarcloud
Copy link

sonarcloud bot commented Aug 13, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

32.7% 32.7% Coverage
4.0% 4.0% Duplication

@ownclouders
Copy link
Contributor

Results for oC10SharingInternalGroupsSharingIndicator https://drone.owncloud.com/owncloud/web/18259/24/1
The following scenarios passed on retry:

  • webUISharingInternalGroupsToRootSharingIndicator/shareWithGroups.feature:104

@ownclouders
Copy link
Contributor

Results for oC10SharingExternal https://drone.owncloud.com/owncloud/web/18259/39/1
The following scenarios passed on retry:

  • webUISharingExternal/federationSharing.feature:363

@rpocklin
Copy link
Contributor Author

This PR is ready for review, not sure how coverage is calculated in SonarCloud but this is outside the source code of the project files.

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Good idea 👍🏻

@kulmann kulmann merged commit 368fe0f into owncloud:master Aug 13, 2021
@@ -0,0 +1,6 @@
Change: Enable live reload for changes to themes
Copy link
Contributor

Choose a reason for hiding this comment

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

Love it! Not a "Change" in terms of semver from my POV, though ;)

Copy link
Member

Choose a reason for hiding this comment

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

True, changed it to enhancement now, thanks for the hint!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, agreed!

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.

4 participants