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

Save custom styles in DSpace Admin collection form and have it dynamically added to <head> for the collection and item pages #3250

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

alexklbuckley
Copy link

@alexklbuckley alexklbuckley commented Aug 13, 2024

References

Add references/links to any related issues or PRs. These may include:

Description

Add ability for DSpace admin users to save custom CSS rules for a collection.

Render that CSS on the collection browse and item detail pages.

Instructions for Reviewers

List of changes in this PR:

  • Add new metadata field: collection.css
  • Add new function to collection.model.ts to fetch content of new metadata field
  • Add a new function to theme.service.ts: addCollectionCSSToHead()
  • Call new addCollectionCSSToHead() function whenever the user is viewing a collection page OR is viewing an item.

Include guidance for how to test or review your PR.

  1. Log into DSpace Admin
  2. Go to: Registries > Metadata
  3. Create a new metadata schema: Namespace = 'collection', Name = 'collection'
  4. Add a new field to your new schema: Element = 'css', Qualifier = empty, Scope note = 'CSS for this collection'
  5. Go to: Edit > Collection
  6. Save the following CSS in the 'Collection styles (CSS)' input field:
    body {
    background-color:red !important;
    }
    .bottom-footer {
    background-color: yellow !important;
    }
    header {
    background-color: green !important;
    }
  7. The Collection Browse page will load. Notice your CSS styles have been applied to the header, body and footer of the page.
  8. Click on an item record. Notice the same CSS styles have been applied on that page as well.
  9. Go to: Edit > Collection.
  10. Choose a different collection and save the following CSS styling in the 'Collection styles (CSS)' field:
    body {
    background-color:orange !important;
    }
  11. The Collection Browse page for this second collection will load. Notice that your second set of styling is displaying.
  12. View an item in this second collection. Notice it has an orange background color.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

dynamically added to <head> for the collection and item pages

Test plan:
1. Log into DSpace Admin
2. Go to: Edit > Collection
3. Save the following CSS in the 'Collection styles (CSS)' input field:
body {
    background-color:red !important;
}
.bottom-footer {
    background-color: yellow !important;
}
header {
    background-color: green !important;
}
.navbar-inner-container {
    background-color: brown !important;
}
4. The Collection Browse page will load. Hard refresh and notice your
   CSS styles have been applied to the header, nav, body and footer of
   the page.
5. Click on an item record and (if neccessary hard refresh again)
   notice the same CSS styles have been applied on that page as well.
6. Go to: Edit > Collection.
7. Choose a different collection and save the following CSS styling in
   the 'Collection styles (CSS)' field:
body {
    background-color:orange !important;
}
8. Save and hard refresh the Browse collection page that is loaded and
   hard refresh the Browse collection page from step 4. Notice that
   different collections successfully render the different CSS styling.
   i.e. you are not saving global CSS but collection specific CSS.
9. Notice that this applies to the item pages of the two different
   collections as well.

Sponsored-by: Auckland University of Technology, New Zealand
- Fix tests

Sponsored-by: Auckland University of Technology, New Zealand
alexklbuckley and others added 2 commits August 28, 2024 17:08
- Trying to fix tests

Sponsored-by: Auckland University of Technology, New Zealand
@alexklbuckley alexklbuckley force-pushed the dspace-css branch 5 times, most recently from 20471cb to 7f36ee0 Compare September 2, 2024 23:42
- Fixing failing unit tests

Sponsored-by: Auckland University of Technology, New Zealand
alexklbuckley added a commit to alexklbuckley/DSpace that referenced this pull request Sep 3, 2024
- This commit adds the metadata fields to the existing dspace metadata
  schema required for this DSpace Angular PR#3250:  DSpace/dspace-angular#3250

Sponsored-by: Auckland University of Technology, New Zealand
@kshepherd
Copy link
Member

kshepherd commented Sep 7, 2024

Hi @alexklbuckley , thanks for opening this PR -- this is a creative approach to collection styling but I think it raises a few questions about separation of concerns (theme / style vs stored metadata), and possible security issues with injecting metadata text values directly into page output as css. It also means the CSS can't be minified and bundled up with the webpack when delivering the production Angular application.

(I see this PR is in draft - this comment isn't a "review", just trying to help out in case we can find more suitable ways of achieving what you are after)

There are a few other approaches to "collection styling" I can think of, one of which is already supported:

  1. Since DSpace 7.2, it is possible to configure and extend themes for particular handles or uuids (and child objects), see https://wiki.lyrasis.org/display/DSDOC8x/User+Interface+Configuration - extending a theme would mean you didn't have to implement an entire new theme for each distinct collection style, just override the particular component scss you wanted to apply

e.g.

  # # A theme with a handle property will match the community, collection or item with the given
  # # handle, and all collections and/or items within it
  # - name: 'custom',
  #   handle: '10673/1233'
  #
  # # A theme with a regex property will match the route using a regular expression. If it
  # # matches the route for a community or collection it will also apply to all collections
  # # and/or items within it
  # - name: 'custom',
  #   regex: 'collections\/e8043bc2.*'
  #
  # # A theme with a uuid property will match the community, collection or item with the given
  # # ID, and all collections and/or items within it
  # - name: 'custom',
  #   uuid: '0958c910-2037-42a9-81c7-dca80e3892b4'
  #
  # # The extends property specifies an ancestor theme (by name). Whenever a themed component is not found
  # # in the current theme, its ancestor theme(s) will be checked recursively before falling back to default.
  # - name: 'custom-A',
  #   extends: 'custom-B',
  #   # Any of the matching properties above can be used
  #   handle: '10673/34'
  #
  # - name: 'custom-B',
  #   extends: 'custom',
  #   handle: '10673/12'
  1. If the above is still not suitable for this use case, I would consider storing a simple class name in the metadata value rather than directly storing the CSS - that way, the collection page component could at least just validate and inject the class name to any components/elements within that template and the SCSS itself just contains matches for various classes which are effectively "collection theme names". Admins would still be able to set values, but there would be a bit more safety and control over the possible set of CSS styles that can be applied...

I would recommend giving option 1 a go before spending too much time on this solution as I think it could be a hard sell for merging to DSpace. Please let me know if you have any questions or extra info to help clarify if I have misunderstood.

@kshepherd
Copy link
Member

(I just read the original issue and now realise the theme idea was already considered, sorry for missing that. If the key requirement is for an admin without Angular theme access to change styles on demand, then perhaps my suggestions are not as applicable)

@alexklbuckley
Copy link
Author

(I just read the original issue and now realise the theme idea was already considered, sorry for missing that. If the key requirement is for an admin without Angular theme access to change styles on demand, then perhaps my suggestions are not as applicable)

Hi @kshepherd ,

Thanks very much for your suggestions and for your feedback. Appreciate it!

Yes, our use case is to enable librarians, without direct server access, to be able to save and change styling for collections in the DSpace Admin.

I also work on Koha and we have a similar setting for librarians to be able to save CSS rules, in the Koha interface, in a setting called OpacUsercss:
https://koha-community.org/manual/24.05/en/html/opacpreferences.html#opacusercss-label .
For example, like this https://youtu.be/H8cXXluZJmc?t=128

Can you see a means for us to be able to safety save, and display, CSS styles on demand without Angular theme access?
Am I right in thinking that limiting the ability to edit the dspace.collection.css metadata field to administrators helps mitigate some of the safety concerns?


I also have a second PR for saving new additional metadata content fields for collections and then displaying them on an optional collection home page (displayed by default and only routed to when enabled in the config.*.yml file):

The use case for this is enabling collection owners to save, and display, more contextual information about their collection, without needing server access. The fields on this PR accept, and display HTML, but in the same way as the dc.description (Collection 'Introductory text (HTML)') metadata field currently does in DSpace 7 and 8.

I would value your opinion on this PR3256 if you have a chance to take a look.

Thanks so much,
Alex

@tdonohue
Copy link
Member

tdonohue commented Sep 9, 2024

@alexklbuckley & @kshepherd : Just glanced at this code, and I have a few immediate concerns:

  • I don't see anything that protects this dspace.collection.css field to make it only editable to Administrators. So, this means, by default it would also be editable to Community Admins and Collection Admins. That's much more dangerous, as some institutions delegate Community/Collection Admins to many individuals...if one of them is compromised, then someone can use their login to compromise the site.
  • There's no validation that the text entered in this field is CSS (whereas, at least from the Koha demo, it looks like they are validating the entry). This means it could be anything added in this tag. There's a risk that a user could enter in something foolish like body { display: none;} which would then block them from accessing everything (and no one would be able to fix the issue as that Collection would no longer be editable from the UI). This essentially means we have to "trust" the user is entering valid, non-dangerous CSS. We also have trust the browser won't display/run any Javascript or HTML which users may try to add into this field.

Even if we fix these issues, this feature still makes me a bit nervous. Anytime you allow for injecting code into the display of a page, there's risk that injected data could be used to steal user data. While I realize creating a single CSS text area is the easy solution, it's also the most risky... the only way I know of to minimize this risk is to be extremely strict around validating the data entered.

Another idea (but it's much more complex) could be to allow sites to only change the value of various existing Bootstrap CSS variables to allow them control over the basic look & feel without allowing them to add in any CSS they want. This would be a more complex form though, as you'd have to list each of these variables with their current value, and allow users to edit them.

…nistrators

- Add a new 'Edit collection styles' tab to the Edit Collection DSpace Admin forms. This tab page can only be loaded by administrator users

Sponsored-by: Auckland University of Technology, New Zealand
@alexklbuckley
Copy link
Author

@alexklbuckley & @kshepherd : Just glanced at this code, and I have a few immediate concerns:

* I don't see anything that protects this `dspace.collection.css` field to make it only editable to Administrators.  So, this means, by default it would _also_ be editable to Community Admins and Collection Admins.  That's much more dangerous, as some institutions delegate Community/Collection Admins to many individuals...if one of them is compromised, then someone can use their login to compromise the site.

* There's no validation that the text entered in this field is CSS (whereas, at least from the Koha demo, it looks like they are validating the entry).  This means it could be anything added in this tag. There's a risk that a user could enter in something foolish like `body { display: none;}` which would then **block them** from accessing everything (and no one would be able to fix the issue as that Collection would no longer be editable from the UI).  This essentially means we have to "trust" the user is entering valid, non-dangerous CSS. We also have trust the browser won't display/run any Javascript or HTML which users may try to add into this field.

Even if we fix these issues, this feature still makes me a bit nervous. Anytime you allow for injecting code into the display of a page, there's risk that injected data could be used to steal user data. While I realize creating a single CSS text area is the easy solution, it's also the most risky... the only way I know of to minimize this risk is to be extremely strict around validating the data entered.

Another idea (but it's much more complex) could be to allow sites to only change the value of various existing Bootstrap CSS variables to allow them control over the basic look & feel without allowing them to add in any CSS they want. This would be a more complex form though, as you'd have to list each of these variables with their current value, and allow users to edit them.

Thank you @tdonohue and @kshepherd we are working on adding those first two points to this pull request.

I've chatted with my team and we do not think, at this stage, it will be practical to maintain enabling librarians to change the value of various existing bootstrap css variables. For one thing, due to the maintainability of that solution whenever bootstrap css variables changes then the collection forms would have to as well.

In the meantime, if you had availability to take a look at our second pull request, extending the already existing DSpace ability to save text & HTML content in collection metadata fields we would love to get more feedback and testing: #3256

…nistrators

- Fix failing unit tests

Sponsored-by: Auckland University of Technology, New Zealand
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admin user can save custom CSS styles for a collection in DSpace Admin interface
3 participants