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

Use new table icons in Workbench column headers #5176

Draft
wants to merge 8 commits into
base: production
Choose a base branch
from
+12 −22
Draft

Use new table icons in Workbench column headers #5176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2da030c
use new table icons in workbench column headers
sharadsw Aug 5, 2024
f275233
Lint code with ESLint and Prettier
sharadsw Aug 5, 2024
cdc8fbe
remove conditional call
sharadsw Aug 6, 2024
7de7d16
Lint code with ESLint and Prettier
sharadsw Aug 6, 2024
a9576b8
remove ban legacy icon
sharadsw Aug 6, 2024
1c61d37
force svg text vertical alignment
sharadsw Aug 6, 2024
0be9725
Lint code with ESLint and Prettier
sharadsw Aug 6, 2024
6d60f82
Merge branch 'production' into issue-2864
grantfitzsimmons Aug 19, 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
1 change: 0 additions & 1 deletion specifyweb/frontend/js_src/lib/components/Atoms/Icons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export const iconClassName = 'w-6 h-6 flex-shrink-0';
*/
export const legacyNonJsxIcons = {
arrowsExpand: `<svg class="${iconClassName}" aria-hidden="true" fill="currentColor" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" d="M3 4a1 1 0 011-1h4a1 1 0 010 2H6.414l2.293 2.293a1 1 0 11-1.414 1.414L5 6.414V8a1 1 0 01-2 0V4zm9 1a1 1 0 010-2h4a1 1 0 011 1v4a1 1 0 01-2 0V6.414l-2.293 2.293a1 1 0 11-1.414-1.414L13.586 5H12zm-9 7a1 1 0 012 0v1.586l2.293-2.293a1 1 0 111.414 1.414L6.414 15H8a1 1 0 010 2H4a1 1 0 01-1-1v-4zm13-1a1 1 0 011 1v4a1 1 0 01-1 1h-4a1 1 0 010-2h1.586l-2.293-2.293a1 1 0 111.414-1.414L15 13.586V12a1 1 0 011-1z" clip-rule="evenodd"></path></svg>`,
ban: `<svg class="${iconClassName}" aria-hidden="true" fill="currentColor" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" d="M13.477 14.89A6 6 0 015.11 6.524l8.367 8.368zm1.414-1.414L6.524 5.11a6 6 0 018.367 8.367zM18 10a8 8 0 11-16 0 8 8 0 0116 0z" clip-rule="evenodd" /></svg>`,
link: `<svg class="${iconClassName}" aria-hidden="true" fill="currentColor" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg"><path d="M11 3a1 1 0 100 2h2.586l-6.293 6.293a1 1 0 101.414 1.414L15 6.414V9a1 1 0 102 0V4a1 1 0 00-1-1h-5z" /><path d="M5 5a2 2 0 00-2 2v8a2 2 0 002 2h8a2 2 0 002-2v-3a1 1 0 10-2 0v3H5V7h3a1 1 0 000-2H5z" /></svg>`,
printer: `<svg class="${iconClassName}" aria-hidden="true" fill="currentColor" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" d="M5 4v3H4a2 2 0 00-2 2v3a2 2 0 002 2h1v2a2 2 0 002 2h6a2 2 0 002-2v-2h1a2 2 0 002-2V9a2 2 0 00-2-2h-1V4a2 2 0 00-2-2H7a2 2 0 00-2 2zm8 0H7v3h6V4zm0 8H7v4h6v-4z" clip-rule="evenodd" /></svg>`,
} as const;
Expand Down
2 changes: 2 additions & 0 deletions specifyweb/frontend/js_src/lib/components/Molecules/SvgIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export function SvgIcon({
</g>
<g>
<text
alignmentBaseline="middle"
baselineShift="-10%"
Comment on lines +63 to +64
Copy link
Contributor Author

@sharadsw sharadsw Aug 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The svg text does not vertically align without this for column headers. The baselineShift is a magic number but it perfectly aligns the text. Without it, the vertically alignment is juuust so slightly to the top. (this is so silly)

Without alignmentBaseline and baselineShift:
Screenshot 2024-08-06 at 10 33 55 AM

With both:
Screenshot 2024-08-06 at 10 48 25 AM

Without baselineShift:
image

Copy link
Member

Choose a reason for hiding this comment

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

I am ok with this fix, but I would like you to double check one thing:
The svg icons work fine everywhere else, but break in this one place.
I suspect thus the issue is not with the SVG icons, but with the interference with the handsontable styling.
could you verify if some style from handsontable is interfering with our icons? (use the styles panel in the dev tools for that)

then, if you find the cause, you can add the fixup CSS into this file:
https://github.com/specify/specify7/blob/production/specifyweb/frontend/js_src/css/workbench.css

I leave it up to you to pick which solution you prefer more

dominantBaseline="central"
fill="#FFFFFF"
fontFamily="Francois One,sans-serif"
Expand Down
31 changes: 10 additions & 21 deletions specifyweb/frontend/js_src/lib/components/WorkBench/hotProps.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ import React from 'react';
import ReactDOMServer from 'react-dom/server';

import { wbPlanText } from '../../localization/wbPlan';
import { f } from '../../utils/functools';
import { icons } from '../Atoms/Icons';
import { getTable } from '../DataModel/tables';
import { TableIcon } from '../Molecules/TableIcon';
import { userPreferences } from '../Preferences/userPreferences';
import type { Dataset } from '../WbPlanView/Wrapped';
import type { WbMapping } from './mapping';
Expand Down Expand Up @@ -62,23 +61,19 @@ export function useHotProps({

const colHeaders = React.useCallback(
(physicalCol: number) => {
const tableIcon = mappings?.mappedHeaders?.[physicalCol];
const isMapped = tableIcon !== undefined;
const tableIconUrl = mappings?.mappedHeaders?.[physicalCol];
const isMapped = tableIconUrl !== undefined;
const mappingCol = physicalColToMappingCol(physicalCol);
const tableName =
(typeof mappingCol === 'number'
? mappings?.tableNames[mappingCol]
: undefined) ?? tableIcon?.split('/').slice(-1)?.[0]?.split('.')?.[0];
const tableLabel = isMapped
? f.maybe(tableName, getTable)?.label ?? tableName ?? ''
: '';
// REFACTOR: use new table icons
: undefined) ?? tableIconUrl?.split('/').at(-1)?.split('.')[0];

return ReactDOMServer.renderToString(
<ColumnHeader
columnName={dataset.columns[physicalCol]}
isMapped={isMapped}
tableIcon={tableIcon}
tableLabel={tableLabel}
tableName={tableName}
/>
);
},
Expand Down Expand Up @@ -133,23 +128,17 @@ export function useHotProps({

function ColumnHeader({
isMapped,
tableLabel,
tableIcon,
columnName,
tableName,
}: {
readonly isMapped: boolean;
readonly tableLabel: string;
readonly tableIcon: string | undefined;
readonly columnName: string;
readonly tableName: string | undefined;
}): JSX.Element {
return (
<div className="flex items-center gap-1 pl-4">
{isMapped ? (
<img
alt={tableLabel}
className="w-table-icon h-table-icon"
src={tableIcon}
/>
{isMapped && tableName !== undefined ? (
<TableIcon label={false} name={tableName} />
) : (
<span
aria-label={wbPlanText.unmappedColumn()}
Expand Down
Loading