-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Toggle datatypes for table directly from view dropdown #366
Toggle datatypes for table directly from view dropdown #366
Conversation
can be discussed later
@flashpunch0-0 is attempting to deploy a commit to the dottle's projects Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
) : ( | ||
<i className="bi bi-toggle-off" /> | ||
), | ||
function: () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function: () => { | |
function: () => | |
setSettings((prev) => ({ | |
...prev, | |
showDataTypes: !prev.showDataTypes, | |
})), |
Please make sure to update the state using the setSettings function. Also the build fails here, please take a lokk https://github.com/drawdb-io/drawdb/actions/runs/13877624768/job/38843669905?pr=366
? "(" + fieldData.size + ")" | ||
: "")} | ||
</span> | ||
{settings.showDataTypes && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also try putting the condition before the div so that an emty element doesnt get added to the dom
…e_datatypes_for_table_flashpunch
…ement in table.jsx code in the else part im directly returning the element by keeping a check for settings.showDataTypes? which helps to not add empty element for setSetting issue I used the similar code used just above the snippet and modified it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
@1ilit happy to contribute to this product |
For sure, looking forward to it, thanks for all your work |
* showDatatype setting added in view control panel * removed shortcut can be discussed later * fixed linting issue and made sure the condition check is above div element in table.jsx code in the else part im directly returning the element by keeping a check for settings.showDataTypes? which helps to not add empty element for setSetting issue I used the similar code used just above the snippet and modified it
proposed implementation
1- initially thought of creating a separate context and hook but the settings context did the provide a good base


2- added an attribute(showDataTypes) in settings context
3- in the en.js file created a reference
4- In the ControlPanel.jsx file under view dropdown created the option show Datatype


5- finally in the table.jsx added condition to check settings.showDatatypes and return the element as per condition
working

datatype -> onshow
datatype-> off

Reference issue and pr #346