-
Notifications
You must be signed in to change notification settings - Fork 33
feat: add help cards in /help, based on configurable supportEmail #1933
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @omkar-ethz - I've reviewed your changes - here's some feedback:
- Consider extracting the support contact card into a shared component instead of duplicating the HTML and logic in both Help and About.
- Replace the raw "PSI" string check with a constant or enum to avoid typos and improve maintainability when gating the About card.
- You can remove the wrapping
tag around the mat-card in help.component.html and apply the *ngIf directly on the mat-card to keep the DOM structure cleaner.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the support contact card into a shared component instead of duplicating the HTML and logic in both Help and About.
- Replace the raw "PSI" string check with a constant or enum to avoid typos and improve maintainability when gating the About card.
- You can remove the wrapping <p> tag around the mat-card in help.component.html and apply the *ngIf directly on the mat-card to keep the DOM structure cleaner.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
what's the idea behind having the email in both places? We should remember to document this once this (reviewed and improved) is in this repo's readme/docs |
this was based on Leo's suggestion (that user's might also want to see the support email in about). But i agree the Help page is pretty easy to find, so fine with removing the card from About. |
I will also create a PR in the current version of the docs once we decide the behavior of |
thx. I also think the /help is enough |
Removed the card in /about. Created a PR in docs: SciCatProject/documentation#81 - unfortunately i don't have permissions to add reviewers or merge. Once this PR is merged, will create a PR in scicat-ci's version of frontend. |
minottic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx!
…rtEmail (#1933) This PR adds an environment variable `supportEmail`. If set, a card "Questions or issues" is added in the `/help` page like so:  Currently, users had no way to look up a contact email in case of issues or questions. This PR adds it via a configurable `supportEmail` env var. None Please provide a list of the changes implemented by this PR * add `supportEmail` in appConfig * add a card in /help component - [ ] Included for each change/fix? - [x] Passing? (Merge will not be approved unless this is checked) - [ ] swagger documentation updated \[required\] - [x] official documentation updated \[nice-to-have\] SciCatProject/documentation#81 N/A - [ ] Does it require a specific version of the backend - which version of the backend is required: Add supportEmail configuration and display contextual help cards in the UI New Features: - Introduce supportEmail environment variable in AppConfig - Render a “Questions or issues?” card on the /help page when supportEmail is set - Render a “Getting help” card on the /about page for PSI facility with optional contact email
Description
This PR adds an environment variable

supportEmail. If set, a card "Questions or issues" is added in the/helppage like so:Motivation
Currently, users had no way to look up a contact email in case of issues or questions. This PR adds it via a configurable
supportEmailenv var.Fixes:
None
Changes:
Please provide a list of the changes implemented by this PR
supportEmailin appConfigTests included
Documentation
official documentation info
N/A
Backend version
Summary by Sourcery
Add supportEmail configuration and display contextual help cards in the UI
New Features: