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.

Sign up for GitHub

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

Jump to bottom

Style cleanup on tables and <ActionsDropdown /> #63

Merged
merged 5 commits into from
Apr 24, 2024
Merged

Style cleanup on tables and <ActionsDropdown /> #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87d0086
src/lib-components: styling cleanup on <DynamicTable /> and <ActionsD…
DavidLarsKetch Apr 22, 2024
326039d
Update src/lib-components/lists/DynamicTable.vue
DavidLarsKetch Apr 23, 2024
dfb5af9
src/lib-components/lists/DynamicTable: removes tbody divide line
DavidLarsKetch Apr 23, 2024
7c72b60
src/lib-components/lists/DynamicTable: widens sort click target by in…
DavidLarsKetch Apr 23, 2024
6ebadf6
src/lib-components/lists/DataTable: adds no border, striped rows
DavidLarsKetch Apr 24, 2024
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
16 changes: 10 additions & 6 deletions src/lib-components/lists/DataTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ const { columns, hasActions, isEmptyCellValue, rows } = useTable(
class="overflow-hidden border-b border-gray-200 shadow sm:rounded-lg"
>
<table class="min-w-full divide-y divide-gray-200">
<thead>
<thead class="bg-gray-100">
<tr>
<th
v-for="(col, idx) in columns"
:key="idx"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase bg-gray-50 leading-4"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase leading-4"
:class="col.alignment"
>
{{ col.title }}
Expand All @@ -48,12 +48,16 @@ const { columns, hasActions, isEmptyCellValue, rows } = useTable(
<!--Table Actions Header-->
<th
v-if="hasActions"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase bg-gray-50 leading-4"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase leading-4"
/>
</tr>
</thead>
<tbody class="bg-white divide-y divide-gray-200">
<tr v-for="(row, rowIdx) in rows" :key="rowIdx">
<tbody class="bg-white">
<tr
v-for="(row, rowIdx) in rows"
:key="rowIdx"
class="even:bg-gray-50"
>
<template v-for="(cell, cellIdx) in row.cells" :key="cellIdx">
<component
:is="'td'"
Expand Down Expand Up @@ -87,7 +91,7 @@ const { columns, hasActions, isEmptyCellValue, rows } = useTable(

<tr v-if="rows.length === 0">
<td
:colspan="columns.length"
:colspan="columns.length + (hasActions ? 1 : 0)"
class="px-6 py-4 text-sm text-gray-700 whitespace-nowrap leading-5"
>
No items were found!
Expand Down
109 changes: 57 additions & 52 deletions src/lib-components/lists/DynamicTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,76 +236,81 @@ loadAndRender()
class="relative z-0 min-w-full align-middle border-b border-gray-200 shadow sm:rounded-lg overflow-x-auto"
>
<table class="min-w-full divide-y divide-gray-200">
<thead>
<thead class="bg-gray-100">
<tr>
<th
v-for="(col, idx) in columns"
:key="idx"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase bg-gray-50 leading-4"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase leading-4"
:class="col.alignment"
>
<span v-if="col.title">{{ col.title }}</span>
<span
v-if="col.sort"
class="cursor-pointer"
@click.prevent="handleSort(col.sort as string)"
<div
class="inline-flex items-center gap-2"
:class="{ 'cursor-pointer': col.sort }"
@click.prevent="
col.sort ? handleSort(col.sort as string) : undefined
"
>
<svg
v-if="currentSort !== col.sort"
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
class="h-5 inline"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
stroke-width="2"
d="M8 9l4-4 4 4m0 6l-4 4-4-4"
/>
</svg>
<svg
v-else-if="currentSortDirection == 'desc'"
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20"
fill="currentColor"
class="h-5 inline"
>
<path
fill-rule="evenodd"
d="M14.707 12.707a1 1 0 01-1.414 0L10 9.414l-3.293 3.293a1 1 0 01-1.414-1.414l4-4a1 1 0 011.414 0l4 4a1 1 0 010 1.414z"
clip-rule="evenodd"
/>
</svg>
<svg
v-else
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20"
fill="currentColor"
class="h-5 inline"
>
<path
fill-rule="evenodd"
d="M5.293 7.293a1 1 0 011.414 0L10 10.586l3.293-3.293a1 1 0 111.414 1.414l-4 4a1 1 0 01-1.414 0l-4-4a1 1 0 010-1.414z"
clip-rule="evenodd"
/>
</svg>
</span>
<span v-if="col.title">{{ col.title }}</span>
<span v-if="col.sort">
<svg
v-if="currentSort !== col.sort"
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
class="h-5 inline"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
stroke-width="2"
d="M8 9l4-4 4 4m0 6l-4 4-4-4"
/>
</svg>
<svg
v-else-if="currentSortDirection == 'desc'"
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20"
fill="currentColor"
class="h-5 inline"
>
<path
fill-rule="evenodd"
d="M14.707 12.707a1 1 0 01-1.414 0L10 9.414l-3.293 3.293a1 1 0 01-1.414-1.414l4-4a1 1 0 011.414 0l4 4a1 1 0 010 1.414z"
clip-rule="evenodd"
/>
</svg>
<svg
v-else
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20"
fill="currentColor"
class="h-5 inline"
>
<path
fill-rule="evenodd"
d="M5.293 7.293a1 1 0 011.414 0L10 10.586l3.293-3.293a1 1 0 111.414 1.414l-4 4a1 1 0 01-1.414 0l-4-4a1 1 0 010-1.414z"
clip-rule="evenodd"
/>
</svg>
</span>
</div>
</th>

<!--Table Actions Header-->
<th
v-if="hasActions"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase bg-gray-50 leading-4"
class="px-6 py-3 text-xs font-medium tracking-wider text-gray-900 uppercase leading-4"
/>
</tr>
</thead>

<tbody class="bg-white divide-y divide-gray-200">
<tbody class="bg-white">
<tr
v-for="(row, rowIdx) in rows"
:key="rowIdx"
class="even:bg-gray-50"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm good with making this our default, though I subjectively think we could drop the border between rows when they are striped. The extra border adds to the visual competition. Archive uses tables quite a bit more than Portal, so @profsmallpine should probably weight in on moving all in on striped tables.

I'd expect we'd move the DataTable to the same style as well for consistency.

Copy link
Contributor

@profsmallpine profsmallpine Apr 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't find one as more compelling than the other, but I think it is best to pick one, either using border or stripes. The downside of stripes for me is that it makes even rows the same background color as the header which I find visually distracting to have the same emphasis for different things. Typically, the headers will loose their background shading so that you put the visual emphasis on the differentiation of rows.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How do we feel about this option?

Striped, no border between rows, header to bg-gray-100 from bg-gray-50.
Screenshot 2024-04-23 at 11 02 49 AM

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, should be OK. We use bg-gray-100 as the app shell background color, so outside a card wrapper it might be a touch off, but not to a degree I think we need to sweat right now. I think Portal currently wraps all tables in some kind of card like wrapper.

@click="$emit('click:row', row.rowData)"
>
<template v-for="(cell, cellIdx) in row.cells" :key="cellIdx">
Expand Down Expand Up @@ -341,7 +346,7 @@ loadAndRender()

<tr v-if="!hasContent">
<td
:colspan="rows.length"
:colspan="columns.length + (hasActions ? 1 : 0)"
class="px-6 py-4 text-sm text-gray-700 whitespace-nowrap leading-5"
>
No items were found!
Expand Down
2 changes: 1 addition & 1 deletion src/lib-components/navigation/ActionsDropdown.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const { floatingStyles } = useFloating(trigger, wrapper, {
<Menu as="div" class="relative flex justify-end items-center">
<MenuButton
ref="trigger"
class="w-8 h-8 bg-white inline-flex items-center justify-center text-gray-700 rounded-full hover:text-gray-900 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-blue-500 disabled:opacity-50 disabled:cursor-not-allowed"
class="w-8 h-8 inline-flex items-center justify-center text-gray-700 rounded-full hover:text-gray-900 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-blue-500 disabled:opacity-50 disabled:cursor-not-allowed"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a note: this should be OK. The only other places I recall using ActionsDropdown is in DescriptionLists and on the mastermind group show page. Letting the parent background through, is probably the most desirable style in most use cases.

:disabled="!hasActions"
>
<span class="sr-only">Open options</span>
Expand Down
Loading