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

Editor and save state tweaks #879

Merged
merged 4 commits into from
Sep 1, 2022
Merged

Editor and save state tweaks #879

merged 4 commits into from
Sep 1, 2022

Conversation

apedroferreira
Copy link
Member

Implements PR feedback from #861.
No debug logs, no color in logs, less log messages, no hiding of save state icon
Also adjusted a section title in the page properties editor, to improve consistency with changes in the component properties editor.

@apedroferreira apedroferreira self-assigned this Sep 1, 2022
@render
Copy link

render bot commented Sep 1, 2022

@oliviertassinari oliviertassinari requested a deployment to editor-tweaks-1-sept - toolpad-db PR #879 September 1, 2022 13:41 — with Render Abandoned
@apedroferreira
Copy link
Member Author

apedroferreira commented Sep 1, 2022

The console logs are making the CI tests fail. Any suggestions since we don't use the env variable?
Should i try to disable console logs in tests?

Edit: disabling logs in tests would probably not be good, I've added a check for the test environment around the log

@Janpot
Copy link
Member

Janpot commented Sep 1, 2022

The console logs are making the CI tests fail.

Only the use of console.error should make it fail at the moment. In general, we should avoid these sort of checks for test env in the app. There is no reason why the integration test should fail on this, so if the log is a problem for the current tests, then the tests need to change, not the app.

edit: btw, There is an ignore list for logs in the tests, but it doesn't matter here, because the tests shouldn't break on the use of console.log

@apedroferreira
Copy link
Member Author

sounds good i probably misunderstood why the ci failed then

@Janpot
Copy link
Member

Janpot commented Sep 1, 2022

creating #881 for more insight in these messages

@apedroferreira
Copy link
Member Author

apedroferreira commented Sep 1, 2022

creating #881 for more insight in these messages

it just passed now without any new changes, not sure what had happened before, it did say something about console logs

@apedroferreira apedroferreira merged commit 9e28902 into master Sep 1, 2022
@apedroferreira apedroferreira deleted the editor-tweaks-1-sept branch September 1, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants