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.

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

scaffold: Use checkbox to display boolean value #732

Merged
merged 4 commits into from
Jun 24, 2020

Conversation

Tobbe
Copy link
Member

@Tobbe Tobbe commented Jun 21, 2020

Previously in the scaffolded display table date/time values were wrapped in a timeTag function before displaying them, and all other values were wrapped in truncate. I made the name of that display function part of the metadata we have about components in scaffold.js, so we don't need a big if-statement in the actual template for scaffolding the table.

After that refactoring I also added a new display function for displaying boolean values using a checkbox

image

@Tobbe Tobbe force-pushed the tobbe-scaffolds-display-checkbox branch from cce30c8 to 700da4c Compare June 21, 2020 12:44
@Tobbe Tobbe force-pushed the tobbe-scaffolds-display-checkbox branch from 700da4c to fe97b12 Compare June 21, 2020 12:53
@Tobbe Tobbe mentioned this pull request Jun 21, 2020
@thedavidprice
Copy link
Contributor

Hi @Tobbe I took a quick glance at this and one question I have is regarding what (if any) issues might this cause for existing Apps with CRUD when they upgrade to use this? I don't think there would be an issue with the checkbox itself since it's new. But if the scaffold.css already exists and someone 1) upgrades and 2) generates new CRUD for a model including boolean (which won't overwrite scaffold.css), then there might be a display issue due to not having the new and improved .css. Am I thinking that through correctly?

To be clear, this is definitely not a dealbreaker. But if we can anticipate what the issues might be then I'd like to include info in the next Release Notes and/or create a Forum Topic (or just refer to this one). Make sense?

@Tobbe
Copy link
Member Author

Tobbe commented Jun 24, 2020

@thedavidprice Ahh, didn't realize scaffold.css wasn't updated if it already exists. I totally understand why that it the case though.

For this PR it's not a huge deal. Without the css changes, the only difference will be that the checkbox is shifted four pixels to the right (in Chrome that is. Other browsers might have other default styles).

With css changes
image

Without css changes
image

So people who update, and then generate CRUD for boolean values, will get the styling from the second image above.
And the fix, if you want to mention that, is to manually insert the css rule this PR introduces

@cannikin cannikin merged commit 5e51421 into redwoodjs:main Jun 24, 2020
@thedavidprice thedavidprice added this to the next release milestone Jun 24, 2020
@Tobbe Tobbe deleted the tobbe-scaffolds-display-checkbox branch June 24, 2020 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants