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

Improve look of models in Browse data #38518

Merged
merged 10 commits into from
Feb 8, 2024
1 change: 1 addition & 0 deletions .github/workflows/uberjar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:
push:
branches:
- "master"
- "models-in-browse-data-polish"
rafpaf marked this conversation as resolved.
Show resolved Hide resolved
paths-ignore:
# config files
- ".**"
Expand Down
16 changes: 9 additions & 7 deletions frontend/src/metabase/browse/components/BrowseModels.styled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@ export const ModelCard = styled(Card)`
align-items: flex-start;

border: 1px solid ${color("border")};
box-shadow: 0 1px 0.25rem 0 rgba(0, 0, 0, 0.06);

box-shadow: none;
&:hover {
box-shadow: 0 1px 0.25rem 0 rgba(0, 0, 0, 0.14);
h4 {
h1 {
color: ${color("brand")};
}
}
transition: box-shadow 0.15s;
h4 {

h1 {
transition: color 0.15s;
}
`;
Expand All @@ -50,8 +51,9 @@ export const MultilineEllipsified = styled(Ellipsified)`

export const GridContainer = styled.div`
display: grid;
grid-template-columns: repeat(auto-fill, minmax(15rem, 1fr));
gap: 1rem;
grid-template-columns: repeat(auto-fill, minmax(20rem, 1fr));
gap: 1.5rem 1rem;
margin-top: 1rem;
width: 100%;

${breakpointMinSmall} {
Expand All @@ -65,7 +67,7 @@ export const GridContainer = styled.div`
export const CollectionHeaderContainer = styled.div`
grid-column: 1 / -1;
align-items: center;
padding-top: 0.5rem;
padding-top: 1rem;
margin-right: 1rem;
&:not(:first-of-type) {
border-top: 1px solid #f0f0f0;
Expand Down
26 changes: 11 additions & 15 deletions frontend/src/metabase/browse/components/BrowseModels.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import _ from "underscore";
import cx from "classnames";
import { c, t } from "ttag";
import { t } from "ttag";

import type {
Card,
Expand Down Expand Up @@ -140,29 +139,24 @@ const ModelCell = ({ model, collectionHtmlId }: ModelCellProps) => {
model.last_editor_common_name ?? model.creator_common_name;
const timestamp = model.last_edited_at ?? model.created_at ?? "";

const noDescription = c(
"Indicates that a model has no description associated with it",
).t`No description.`;
// const noDescription = c(
// "Indicates that a model has no description associated with it",
// ).t`No description.`;
rafpaf marked this conversation as resolved.
Show resolved Hide resolved
return (
<Link
aria-labelledby={`${collectionHtmlId} ${headingId}`}
key={model.id}
to={Urls.model(model as unknown as Partial<Card>)}
>
<ModelCard>
<Title order={4} className="text-wrap" lh="1rem" mb=".5rem">
<Box mb="auto">
<Icon name="model" size={20} className="text-brand" />
rafpaf marked this conversation as resolved.
Show resolved Hide resolved
</Box>
<Title className="text-wrap" lh="1rem" mb=".25rem" size="1rem">
Copy link
Contributor

Choose a reason for hiding this comment

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

let's not use a className, also I don't think it does anything

Suggested change
<Title className="text-wrap" lh="1rem" mb=".25rem" size="1rem">
<Title lh="1rem" mb=".25rem" size="1rem">

<MultilineEllipsified tooltipMaxWidth="20rem" id={headingId}>
{model.name}
</MultilineEllipsified>
</Title>
<Text h="2rem" size="xs" mb="auto">
<MultilineEllipsified
tooltipMaxWidth="20rem"
className={cx({ "text-light": !model.description })}
>
{model.description || noDescription}{" "}
</MultilineEllipsified>
</Text>
<LastEdited editorFullName={lastEditorFullName} timestamp={timestamp} />
</ModelCard>
</Link>
Expand All @@ -182,7 +176,9 @@ const CollectionHeader = ({
<CollectionHeaderLink to={Urls.collection(collection)}>
<Group spacing=".25rem">
<Icon name="folder" color="text-dark" size={16} />
<Text weight="bold">{getCollectionName(collection)}</Text>
<Text weight="bold" color="text-medium">
{getCollectionName(collection)}
</Text>
</Group>
</CollectionHeaderLink>
</CollectionHeaderGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,30 @@ function MainNavbarView({
>
{t`Home`}
</PaddedSidebarLink>
{hasDataAccess && (
<>
<PaddedSidebarLink
icon="database"
url={BROWSE_URL}
isSelected={nonEntityItem?.url?.startsWith(BROWSE_URL)}
onClick={onItemSelect}
>
{t`Browse data`}
</PaddedSidebarLink>
{!hasOwnDatabase && isAdmin && (
<AddYourOwnDataLink
icon="add"
url={ADD_YOUR_OWN_DATA_URL}
isSelected={nonEntityItem?.url?.startsWith(
ADD_YOUR_OWN_DATA_URL,
)}
onClick={onItemSelect}
>
{t`Add your own data`}
</AddYourOwnDataLink>
)}
</>
)}
</ul>
</SidebarSection>

Expand Down Expand Up @@ -130,36 +154,6 @@ function MainNavbarView({
aria-label="collection-tree"
/>
</SidebarSection>

{hasDataAccess && (
<SidebarSection>
<SidebarHeadingWrapper>
<SidebarHeading>{t`Data`}</SidebarHeading>
</SidebarHeadingWrapper>
<ul>
<PaddedSidebarLink
icon="database"
url={BROWSE_URL}
isSelected={nonEntityItem?.url?.startsWith(BROWSE_URL)}
onClick={onItemSelect}
>
{t`Browse data`}
</PaddedSidebarLink>
{!hasOwnDatabase && isAdmin && (
<AddYourOwnDataLink
icon="add"
url={ADD_YOUR_OWN_DATA_URL}
isSelected={nonEntityItem?.url?.startsWith(
ADD_YOUR_OWN_DATA_URL,
)}
onClick={onItemSelect}
>
{t`Add your own data`}
</AddYourOwnDataLink>
)}
</ul>
</SidebarSection>
)}
</div>
<WhatsNewNotification />
</SidebarContentRoot>
Expand Down