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): Adding Explore all button on home page search #6468

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

jjoyce0510
Copy link
Collaborator

Summary

Adding a button for browsing all entities from the Home page. This allows users to start exploring before they know exactly what exists inside of DataHub.

Demo

Screen.Recording.2022-11-16.at.9.57.02.PM.mov

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 the product PR or Issue related to the DataHub UI/UX label Nov 17, 2022
@github-actions
Copy link

Unit Test Results (build & test)

622 tests  ±0   618 ✔️ ±0   15m 58s ⏱️ -7s
157 suites ±0       4 💤 ±0 
157 files   ±0       0 ±0 

Results for commit b4134ed. ± Comparison against base commit 2a791d4.

Copy link
Collaborator

@chriscollins3456 chriscollins3456 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! i have a couple of nits and just a general css question in our frontend, but nothing blocking me from approving

datahub-web-react/src/app/search/SearchBar.tsx Outdated Show resolved Hide resolved
datahub-web-react/src/app/home/HomePageHeader.tsx Outdated Show resolved Hide resolved
Comment on lines +117 to +121
&& {
padding: 0px;
margin: 0px;
height: 16px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

just in general I'm confused why we have these && or &&& all over our css styling... do you understand it?

at least to my understanding, using & is meant to apply styles to the current selector you're on as well as above selectors, so idk if it's really doing anything in these situations, but I could be wrong since I've never seen it used other than with &:hover { ... or some pseudo selector

also I recognize you may be copy pasting this styling from elsewhere lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no so for some reason i have to do this to bypass the native antd styles in some cases. would love to understand why

Copy link
Collaborator

Choose a reason for hiding this comment

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

fricken ant design man

@jjoyce0510 jjoyce0510 merged commit added63 into datahub-project:master Nov 17, 2022
cccs-Dustin pushed a commit to CybercentreCanada/datahub that referenced this pull request Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants