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

feat(ui) Show Acryl information with button and banner behind flag #8330

Merged

Conversation

chriscollins3456
Copy link
Collaborator

Creates a few desired Managed DataHub / Acryl CTAs that are hidden behind a feature flag that is default off. We will turn this on in our demo site to show a button to book a demo and a banner on the home page.

image image

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX labels Jun 28, 2023
Copy link
Contributor

@joshuaeilers joshuaeilers left a comment

Choose a reason for hiding this comment

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

Looks good. Only one potential issue with the feature flag default, otherwise looks great.

Comment on lines +11 to +12
color: #262626;
background-color: #e6f4ff;
Copy link
Contributor

Choose a reason for hiding this comment

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

May be nice to put these into a constants, but I bet they're only used here for now. If so, it's fine the way it is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ugh yeah these are only used here right now but I bet they may be involved in the color overhaul that's coming

`;

const StyledLink = styled(Link)`
color: #1890ff;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question about this color.

Comment on lines +11 to +14
export function useIsShowAcrylInfoEnabled() {
const appConfig = useAppConfig();
return appConfig.config.featureFlags.showAcrylInfo;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for making this a shared hook ❤️

@@ -289,6 +289,7 @@ featureFlags:
showBrowseV2: ${SHOW_BROWSE_V2:true} # Enables showing the browse v2 sidebar experience.
preProcessHooks:
uiEnabled: ${PRE_PROCESS_HOOKS_UI_ENABLED:true} # Circumvents Kafka for processing index updates for UI changes sourced from GraphQL to avoid processing delays
showAcrylInfo: ${SHOW_ACRYL_INFO:true} # Show different CTAs within DataHub around moving to Managed DataHub. Set to true for the demo site.
Copy link
Contributor

Choose a reason for hiding this comment

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

Default to false here probably?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh man amazing call haha woops! i forgot to switch it back

@chriscollins3456 chriscollins3456 merged commit c051ea0 into datahub-project:master Jun 29, 2023
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants