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

Homepage UI #3711

Merged
merged 19 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions mathesar/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from mathesar.rpc.databases import list_ as databases_list
from mathesar.rpc.schemas import list_ as schemas_list
from mathesar.rpc.servers import list_ as get_servers_list
from mathesar.api.serializers.databases import TypeSerializer
from mathesar.api.serializers.tables import TableSerializer
from mathesar.api.serializers.queries import QuerySerializer
Expand Down Expand Up @@ -83,6 +84,7 @@ def _get_base_data_all_routes(request, database_id=None, schema_id=None):
'current_schema': schema_id,
'current_release_tag_name': __version__,
'databases': get_database_list(request),
'servers': get_servers_list(),
'internal_db_connection': _get_internal_db_meta(),
'is_authenticated': not request.user.is_anonymous,
'queries': [],
Expand Down
8 changes: 4 additions & 4 deletions mathesar_ui/src/api/rpc/database_setup.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { rpcMethodTypeContainer } from '@mathesar/packages/json-rpc-client-builder';

import type { ConfiguredRole } from './configured_roles';
import type { Database } from './databases';
import type { DatabaseResponse } from './databases';
import type { Server } from './servers';

export const sampleDataOptions = [
Expand All @@ -13,15 +13,15 @@ export type SampleDataSchemaIdentifier = (typeof sampleDataOptions)[number];

export interface DatabaseConnectionResult {
server: Server;
database: Database;
database: DatabaseResponse;
configured_role: ConfiguredRole;
}

// eslint-disable-next-line @typescript-eslint/naming-convention
export const database_setup = {
create_new: rpcMethodTypeContainer<
{
database: Database['name'];
database: DatabaseResponse['name'];
sample_data?: SampleDataSchemaIdentifier[];
},
DatabaseConnectionResult
Expand All @@ -31,7 +31,7 @@ export const database_setup = {
{
host: Server['host'];
port: Server['port'];
database: Database['name'];
database: DatabaseResponse['name'];
role: ConfiguredRole['name'];
password: string;
sample_data?: SampleDataSchemaIdentifier[];
Expand Down
89 changes: 86 additions & 3 deletions mathesar_ui/src/api/rpc/databases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,100 @@ import { rpcMethodTypeContainer } from '@mathesar/packages/json-rpc-client-build

import type { Server } from './servers';

export interface Database {
export interface DatabaseResponse {
id: number;
name: string;
server_id: Server['id'];
}

/**
* TODO: Figure out a better place to move classes like these.
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this idea, but I have some different opinions regarding the rules. It would be nice to chat about this on a call.

Also, it's nice seeing this sort of proposal written down, but process-wise I don't like using code comments as a discussion channel. I'd prefer to use a wiki PR or github issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could resolve this in the proposal PRs and on call. I've removed the discussion code comments.

* Perhaps `@mathesar/models/database`.
*
* Model classes should follow these rules:
* - The class should be immutable.
* - There should not be `undefined` properties.
* - All properties should be readonly.
* - All properties should be initialized in the constructor.
* - If a property has to be changed, we should return a new class.
* - Svelte stores should not be used inside Model classes.
* - The Model classes themselves can be contained in a Svelte store.
*
* Good:
* ```
* export class Model {
* readonly proprety: string;
*
* constructor(value: string) {
* this.proprety = value;
* }
*
* fetchAnotherProperty(): string {
* const anotherProperty = api.model.getAnotherProp();
* return anotherProperty;
* }
*
* changeProperty(newValue: string): Model {
* return new Model(newValue);
* }
* }
* ```
*
* Bad:
* ```
* export class Model {
* // All properties should be readonly
* proprety: string;
*
* // Do not have undefined properties
* anotherProp: string | undefined = undefined;
*
* constructor(prop: string) {
* this.proprety = prop;
* }
*
* fetchAnotherProperty(): string {
* // Do not cache here
* this.anotherProp = api.model.getAnotherProp();
* return this.anotherProp;
* }
*
* changeProperty(newValue: string): Model {
* // Do not mutate propreties
* this.property = newValue;
* return this;
* }
* }
* ```
*/
export class Database implements DatabaseResponse {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to put this code somewhere else, and I support putting it in @mathesar/models/database. That move doesn't necessarily need to happen in this PR though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to make any change related to this after we figure out the store structures from the proposal PRs.

readonly id: number;

readonly name: string;

readonly server_id: number;

readonly server_host: string;

readonly server_port: number;
Comment on lines +19 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a single server: Server property instead of three separate server_* properties?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to make any change related to this after we figure out the store structures from the proposal PRs.


constructor(databaseResponse: DatabaseResponse, server: Server) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why you chose a class for Database (instead of an interface). I'd be inclined to use an interface since they're simpler and we don't have any methods. Maybe you foresee adding methods in the future? With an interface we'd lose the server id validation in the constructor, but that doesn't seem important to me. I don't have a strong opinion here though. Mostly just curious.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could resolve this in the proposal PRs and on call.

this.id = databaseResponse.id;
this.name = databaseResponse.name;
if (databaseResponse.server_id !== server.id) {
throw new Error('Server ids do not match');
}
this.server_id = databaseResponse.server_id;
this.server_host = server.host;
this.server_port = server.port;
}
}

export const databases = {
list: rpcMethodTypeContainer<
{
server_id?: Database['server_id'];
server_id?: DatabaseResponse['server_id'];
},
Array<Database>
Array<DatabaseResponse>
>(),
};
2 changes: 1 addition & 1 deletion mathesar_ui/src/components/AppHeader.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
</LinkMenuItem>
<MenuDivider />
<LinkMenuItem icon={iconConnection} href={HOME_URL}>
{$_('database_connections')}
{$_('databases')}
</LinkMenuItem>
{#if $userProfile.isSuperUser}
<LinkMenuItem
Expand Down
149 changes: 149 additions & 0 deletions mathesar_ui/src/components/Card.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
<script lang="ts">
import { iconMoreActions } from '@mathesar/icons';
import {
DropdownMenu,
makeStyleStringFromCssVariables,
} from '@mathesar-component-library';

export let href: string;
export let ariaLabel: string | undefined = undefined;
export let cssVariables: Record<string, string> | undefined = undefined;

let isHoveringMenuTrigger = false;
let isCardFocused = false;

$: style = cssVariables
? makeStyleStringFromCssVariables(cssVariables)
: undefined;
</script>

<div
class="card"
class:focus={isCardFocused}
class:hovering-menu-trigger={isHoveringMenuTrigger}
{style}
>
<a
class="link passthrough"
{href}
aria-label={ariaLabel}
on:focusin={() => {
isCardFocused = true;
}}
on:focusout={() => {
isCardFocused = false;
}}
>
<div class="top">
<slot />
</div>
{#if $$slots.description}
<div class="description">
<slot name="description" />
</div>
{/if}
</a>
{#if $$slots.menu}
<div
class="menu-container"
on:mouseenter={() => {
isHoveringMenuTrigger = true;
}}
on:mouseleave={() => {
isHoveringMenuTrigger = false;
}}
>
<DropdownMenu
showArrow={false}
triggerAppearance="ghost"
triggerClass="dropdown-menu-button"
closeOnInnerClick={true}
placements={['bottom-end', 'right-start', 'left-start']}
label=""
icon={iconMoreActions}
size="small"
>
<slot name="menu" />
</DropdownMenu>
</div>
{/if}
{#if $$slots.footer}
<div class="footer">
<slot name="footer" />
</div>
{/if}
</div>

<style>
.card {
position: relative;
isolation: isolate;
border-radius: var(--border-radius-l);
border: 1px solid var(--slate-200);
background-color: var(--white);
--padding-v-internal: 1rem;
--padding-h-internal: 1rem;
}
.card.focus {
outline: 2px solid var(--slate-300);
outline-offset: 1px;
border-radius: var(--border-radius-l);
}
.link {
display: grid;
grid-template: auto 1fr auto / 1fr;
border-radius: var(--border-radius-l);
cursor: pointer;
overflow: hidden;
height: 100%;
padding: var(--Card__padding-v, var(--padding-v-internal)) 0;
}
.link:hover {
border-color: var(--slate-500);
background-color: var(--slate-50);
box-shadow: 0 0.2rem 0.4rem 0 rgba(0, 0, 0, 0.1);
}
.top {
overflow: hidden;
font-size: var(--text-size-large);
height: var(--menu-trigger-size, auto);
display: flex;
align-items: center;
padding: 0 var(--Card__padding-h, var(--padding-h-internal));
}
.description:not(:empty) {
padding: var(--size-x-small)
var(--Card__padding-h, var(--padding-h-internal)) 0
var(--Card__padding-h, var(--padding-h-internal));
font-size: var(--text-size-base);
}

.menu-container {
position: absolute;
top: 0;
right: 0;
margin: var(--size-ultra-small);
z-index: 1;
}
.menu-container :global(.dropdown-menu-button) {
width: 100%;
height: 100%;
font-size: var(--text-size-large);
color: var(--slate-500);
display: flex;
flex-direction: row;
justify-content: center;
}
.menu-container :global(.dropdown-menu-button:hover) {
color: var(--slate-800);
background: var(--slate-100);
}
.footer {
position: absolute;
bottom: 0;
right: 0;
width: 100%;
z-index: 1;
cursor: pointer;
}
</style>
4 changes: 3 additions & 1 deletion mathesar_ui/src/components/NameWithIcon.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<Spinner />
{:else}
{#each icons as innerIcon}
<Icon {...innerIcon} size="min(1em, 0.75em + 0.25rem)" />
<Icon size="min(1em, 0.75em + 0.25rem)" {...innerIcon} />
{/each}
{/if}
</span>&nbsp;<span class="name">
Expand All @@ -44,6 +44,7 @@
.icon {
color: var(--icon-color, currentcolor);
opacity: var(--NameWithIcon__icon-opacity, 0.75);
vertical-align: middle;
}
.icon > :global(.fa-icon + .fa-icon) {
margin-left: 0.2em;
Expand All @@ -60,5 +61,6 @@
}
.name {
color: var(--name-color, currentcolor);
vertical-align: middle;
}
</style>
10 changes: 7 additions & 3 deletions mathesar_ui/src/components/breadcrumb/BreadcrumbItem.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
import NameWithIcon from '@mathesar/components/NameWithIcon.svelte';
import SchemaName from '@mathesar/components/SchemaName.svelte';
import TableName from '@mathesar/components/TableName.svelte';
import { iconConnection, iconExploration, iconRecord } from '@mathesar/icons';
import {
iconConnectDatabase,
iconExploration,
iconRecord,
} from '@mathesar/icons';
import {
HOME_URL,
getDatabasePageUrl,
Expand All @@ -33,8 +37,8 @@
<DatabaseSelector />
<div class="breadcrumb-item truncate">
<BreadcrumbLink href={HOME_URL}>
<NameWithIcon icon={iconConnection}>
{$_('connections')}
<NameWithIcon icon={{ ...iconConnectDatabase, size: '1.4rem' }}>
{$_('databases')}
</NameWithIcon>
</BreadcrumbLink>
</div>
Expand Down
13 changes: 8 additions & 5 deletions mathesar_ui/src/components/breadcrumb/DatabaseSelector.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { _ } from 'svelte-i18n';

import type { Database } from '@mathesar/api/rpc/databases';
import { iconConnection, iconDatabase } from '@mathesar/icons';
import { iconConnectDatabase, iconDatabase } from '@mathesar/icons';
import { HOME_URL, getDatabasePageUrl } from '@mathesar/routes/urls';
import { databasesStore } from '@mathesar/stores/databases';

Expand All @@ -29,14 +29,17 @@
</script>

<BreadcrumbSelector
data={new Map([[$_('connections'), breadcrumbEntries]])}
triggerLabel={$_('choose_connection')}
data={new Map([[$_('databases'), breadcrumbEntries]])}
triggerLabel={$_('choose_database')}
persistentLinks={[
{
type: 'simple',
label: $_('manage_connections'),
label: $_('manage_databases'),
href: HOME_URL,
icon: iconConnection,
icon: {
...iconConnectDatabase,
size: '1.4rem',
},
// TODO: Handle active states for persistent links
isActive: () => false,
},
Expand Down
Loading
Loading