-
-
Notifications
You must be signed in to change notification settings - Fork 333
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
Type configuration for Text, Number, Boolean, and all mathesar types with single associated db type #926
Conversation
…into type_config
…into type_config
Hey @pavish, I found the title of this PR interesting and would like to know more. Could you comment on why types with one-to-one type mappings require distinct configuration? I've not looked at the code. |
@dmos62 Types with 1-1 mapping do not require a distinct configuration for showing a form and determining db type. They require it for showing what kind of input to use in the table cell, icons, validation of input, formatting the input etc., CellInput is not handled in this PR and would be a subsequent PR. CellInput would also rely on this configuration. |
…into type_config
…into type_config
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.
After playing with these changes, this PR doesn't appear to break any existing functionality.
I've looked over the diff and don't see any glaring red flags.
So for the sake of moving fast, I'm merging this.
That said, there are some smaller concerns and questions with these changes that I plan to address through a combination of tickets, subsequent PRs, and sync conversations.
@seancolsen This PR does not handle saving the user entered input in the forms on the db yet eg., |
Related to:
The PR handles the
database
options in the data type selection dropdown, for Text, Number, Boolean and all mathesar types with a single db type association.The discussion Dynamic Type selection & customization on the frontend #1035 provides the declarative format and additional concerns. I advise the reviewers to review this PR after reading the discussion.
Checklist
Update index.md
).master
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin