Adding Crow's Foot and IDEF1X notation for diagrams#334
Adding Crow's Foot and IDEF1X notation for diagrams#334Francisco-Galindo wants to merge 19 commits intodrawdb-io:mainfrom
Conversation
|
@Francisco-Galindo is attempting to deploy a commit to the dottle's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
nice work, is this pr ready for review? |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@1ilit could you please approve CI to build / deploy? we think this is ready for reciew : ) |
1ilit
left a comment
1ilit
left a comment
There was a problem hiding this comment.
Great work guys, really appreciate it. I'm leaving some comments for now but will take a closer look once these are resolved. Feel free to reach out if you need clarification or have any questions
src/index.css
Outdated
| .table-border { | ||
| border-color: rgba(var(--semi-grey-2), 1); | ||
| /* border-color: rgba(var(--semi-grey-2), 1); */ | ||
| border-color: rgba(0, 255, 0, 1); |
There was a problem hiding this comment.
Is there a reason for this change? Actaully i'm checking now and this class is not used anywhere you can remove it if it has no use
src/i18n/locales/en.js
Outdated
| reset_view: "Reset view", | ||
| show_grid: "Show grid", | ||
| show_cardinality: "Show cardinality", | ||
| default_notation: "Default notation", |
There was a problem hiding this comment.
Let's remove the word 'notation' here so that we have 'Notation'>'Crow's foot' | 'IDEF1X' | 'Default'
src/context/SettingsContext.jsx
Outdated
| panning: true, | ||
| showCardinality: true, | ||
| showRelationshipLabels: true, | ||
| notation: "default", |
There was a problem hiding this comment.
Instead of using string constants here let's have an object like this in constants.js
const Notation = {
DEFAULT: "default",
CROWS_FOOT: "crows_foot",
IDEF1X: "idef1x",
NONE: "none",
};and you can use Notation.DEFAULT, for example, instead of constants
This way we can mimic an enum and have a more unified approach
| notation: { | ||
| children: [ | ||
| { | ||
| default_notation: () => { |
There was a problem hiding this comment.
When you add the Notation object you can reuse the keys here
| idef1x_notation: () => { | ||
| setSettings((prev) => ({ ...prev, notation: "idef1x" })); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Let's also keep the option to not show cardinalities at all. Some people, e.g. me, don't really care about the cardinality and prefer not to set it and not to show it
| </text> | ||
| </> | ||
| )} | ||
| {format(pathRef.current, cardinalityEndX, cardinalityEndY, cardinalityStartX, cardinalityStartY, direction, cardinalityStart, cardinalityEnd)} |
| y2={cardinalityEndY-15} | ||
| stroke="gray" | ||
| strokeWidth='2' | ||
| className="group-hover:fill-sky-700" |
There was a problem hiding this comment.
Hovering over the relationship doesn't make these lines blue:
you'll need to set the stroke color instead of fill to have this take effect
| notation: "default", | ||
| tableWidth: tableWidth, | ||
| showDebugCoordinates: false, | ||
| }; |
There was a problem hiding this comment.
On line 26 add this to prevent any errors that might arise for the cached settings not containing the notation field
setSettings({ ...defaultSettings, ...JSON.parse(settings) });| import { useTranslation } from "react-i18next"; | ||
| import { SideSheet } from "@douyinfe/semi-ui"; | ||
| import RelationshipInfo from "../EditorSidePanel/RelationshipsTab/RelationshipInfo"; | ||
| import { CrowOM, CrowOO, CrowZM, IDEFZM, DefaultNotation } from "./RelationshipFormat"; |
There was a problem hiding this comment.
CromZM is imported but not used, causes linting error. Also remove the function definition if not used
| @@ -10,6 +10,7 @@ const defaultSettings = { | |||
| panning: true, | |||
| showCardinality: true, | |||
5 participants
We want to add both notations to the editor, through a setting so users can select which notation they like best. Adding IDEF1X in particular this would allow our school to use this project as their main DBER editor. Would fix #301.
Support for hierarchical relationships is being worked on, in a non-breaking manner so this doesn't disrupt other people's workflows.
IDEF1X would look like this:
Hierarchies would look something like this:
