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

[Draft] Extend rendering service #5

Closed
wants to merge 2 commits into from

Conversation

abbyhu2000
Copy link
Owner

Description

This PR is a rough draft that extends rendering service to handle some of the duplication branding logic. It proves the logic working for all branding occurrences with default mode and darkmode as well as the option to add a skipvalidation config. It does not include any tests yet.

Here is one potential implementation:

  1. After researching on all the react components that takes in the custom branding configurations, one most duplicated logic is to handle darkmode and default mode. In the rendering service, we can add a function in the rendering service to handle that logic and only return and pass a single URL for the components to use.
  2. Since loading logo logic is more complicated and involve more fallback mechanisms, we should add another function to handle loading logo logic separately in the rendering service. It will return one single URL and also a boolean to indicate if a loading bar should be displayed.
  3. If there is no custom branding, the components will render the default opensearch logo or mark. Since the default opensearch logo/mark/loading logo are different for each branding occurrences, this logic should not be handled in the rendering service, the components would still need to handle their own.
  4. Also explored the option to add in a skipvalidation config. When set to true, it will just pass in original branding configs input by user without any logging or validation.

Issues Resolved

One possible implementation: Extend rendering service
Part of the design spike: opensearch-project#2045

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

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.

1 participant