dspace-angular
dspace-angular copied to clipboard
Replace original selector when making a component themeable
Describe the bug
Our current convention to enable theming for individual components is to
0. For a component Something with the selector ds-something
- Create a
ThemedComponent<Something>wrapper, with the selectords-themed-something - Replace all usages of the original selector with the selector of the new wrapper
This convention can result in a lot of overhead, especially when upgrading existing customizations and themes to a more recent DSpace version
- We essentially need to revisit step (2) for every component that has been made themeable upstream
- It's easy to forget this, and the app will just keep working
- ...but things will be inconsistent, especially if we start theming these newly themeable components
Proposed behavior We could easily circumvent this entire class of customization problems by adopting a different convention when making components themeable:
- Create a
ThemedComponentwrapper for the component, with the same original selector,ds-something - Replace the selector of the unthemed component with
ds-base-something - When creating overriding the component in a theme, use the selector
ds-themed-something- Currently, unthemed and themed components use the same selector. This is possible because these selectors are never used in templates directly
- If we use the original selector for the
ThemedComponentwrapper, we cannot use it for themed components declared in theEagerThemeModule
In this way, all usages of the original unthemed component can stay as-is, and they will be automatically redirected to the themed wrapper without any changes.
Migration If we choose to migrate to this new approach, we'll need to
- Clearly indicate this change in the wiki and changelog.
- Apply the new convention to all existing themed components. I've done this in the past before, as a proof-of-concept, and it was pretty easy to do with a few regex search/replace operations
Additional thoughts:
- For this to work, we'll also need to update the selectors on component overrides in the actual themes
- This will be reflected in the DOM, making it easier to debug which theme ends up being used
Made a proof-of-concept branch by running the ZSH script below and cleaning up a few edge cases where the filenames did not match up correctly
https://github.com/ybnd/dspace-angular/tree/poc-reverse-themed-component-selector-convenction
# Requirements: https://github.com/BurntSushi/ripgrep
# Navigate to a DSpace Angular project before running
def rg-in-place() {
FILE="${@[$#]}"
TEMP="rg-in-place-temp"
rg --passthru $@ > $TEMP && mv $TEMP $FILE
if [[ -f $TEMP ]]; then
rm $TEMP
fi
}
echo "Updating component selectors"
for wrapper in $(rg -g 'themed-*.ts' -l "selector: ?'ds-themed"); do
unthemed=$(echo $wrapper | sed 's/themed-//g')
name=$(basename $unthemed)
echo " $name"
rg-in-place "selector: ?'ds-" -r "selector: 'ds-unthemed-" $unthemed
rg-in-place "selector: ?'ds-themed-" -r "selector: 'ds-" $wrapper
for themed in $(find src/themes -name $name) ; do
if [[ $themed =~ src/themes/([^/]+)/(.*) ]]; then
theme=$match[1]
fi
rg-in-place "selector: ?'ds-" -r "selector: 'ds-$theme-" $themed
done
done
echo "Updating usages in HTML"
for usage in $(rg -g '*.component.html' -l "<ds-themed-?") ; do
echo " $usage"
rg-in-place '<ds-themed-' -r '<ds-' $usage
rg-in-place '</ds-themed-' -r '</ds-' $usage
done
@ybnd and @artlowel : Per today's discussion on this ticket in our DSpace DevMtg, a few notes:
- There seems to be interest in this idea overall, as it seems to make theming easier.
- Concerns expressed though about what the upgrade process might look like for institutions that are using custom themes:
- Will all institutions need to change their themed component selectors to the new model of
ds-[themename]-[component]? Or can their selectors remain as-is? - If all institutions need to change the selectors (or ideally should), then we may need to consider updating the above script to be a
yarnbased tool...that way other institutions could use it to perform these updates in a more automated fashion.
- Will all institutions need to change their themed component selectors to the new model of
- Some were finding it hard to visualize the new theming model... it might be good to add a specific example of how to make an existing component themable using the old way vs the new way. (The current details in the description are not specific enough, and that's where an example could be beneficial.)
Overall, I think this sounds promising. It's unclear to me though whether we will have time/interest to get this into 8.0 quickly. I asked others to comment here though with questions/thoughts to help us come to a quick consensus on direction.
Here's a practical example: let's say we want to make the ChipsComponent themeable and add it to the custom theme
Current process
- Create a themed component wrapper
- File:
src/app/shared/form/chips/themed-chips.component.ts - Class:
ThemedChipsComponent extends ThemedComponent<ChipsComponent> - Selector:
ds-themed-chips - Declared in the same module where
ChipsComponentis declared
- File:
- Change all instances of
ds-chipsin*.htmltods-themed-chips- If we make the component themeable on a fork, we may have to repeat this step if we merge in upstream changes later
- Create a themed version of the component in the
customtheme- File:
src/themes/custom/app/shared/form/chips/chips.component.ts - Import unthemed
src/app/shared/form/chips/chips.component.tsasBaseComponent - Class:
ChipsComponent extends BaseComponent - Selector:
ds-chips - Declared in
LazyThemeModule
- File:
Proposed new process
(changes are highlighted in bold)
- Create a themed component wrapper
- File:
src/app/shared/form/chips/themed-chips.component.ts - Class:
ThemedChipsComponent extends ThemedComponent<ChipsComponent> - Selector:
ds-chips - Declared in the same module where
ChipsComponentis declared
- File:
- Change the selector of
ChipsComponenttods-base-chips - Create a themed version of the component in the
customtheme- File:
src/themes/custom/app/shared/form/chips/chips.component.ts - Import unthemed
src/app/shared/form/chips/chips.component.tsasBaseComponent - Class:
ChipsComponent extends BaseComponent - Selector:
ds-themed-chips - Declared in
LazyThemeModule
- File:
If all institutions need to change the selectors (or ideally should), then we may need to consider updating the above script to be a yarn based tool...that way other institutions could use it to perform these updates in a more automated fashion.
A simple search/replace approach would not be reliable in this case IMO. It's a good start, but makes a lot of assumptions and may require a lot of manual adjustments after -- hard to do when hundreds of files are affected.
Developing a custom linting/fixing setup seems like the best way forward. Moreover, once we have one we can leverage it for a number of other situations any time.