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

New debugInfo and console section #136

Closed
wants to merge 2 commits into from
Closed

New debugInfo and console section #136

wants to merge 2 commits into from

Conversation

LukaszMMazur
Copy link
Contributor

No description provided.

Copy link
Contributor

@mazurroman mazurroman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some minor comments

>
<div className="text-gray-500 dark:text-gray-400 uppercase text-[11px]">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of text-[11px] lets use font-medium to keep consistency with titles in the debug info section

@@ -317,90 +318,101 @@ const Editor = ({ readOnly = false }: Props) => {
>
<div
className={cn(
'w-full md:w-1/2 flex flex-col',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for this change on line 320? It will break the app on mobile screen..

'w-full md:w-1/2 flex flex-col',
isThreeColumnLayout && 'md:w-1/3',
'w-1/2 flex flex-col',
isThreeColumnLayout && 'md:w-2/3',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If isThreeColumtLayout is true, the md:w-1/2 class should be removed.

withLogo={isFullScreen}
/>
</div>
<div className="flex" style={{ height: 'calc(100% - 22vh)' }}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we setting the height here? Can it be removed? We should be able to just use grow to avoid setting any constant values here.

@LukaszMMazur LukaszMMazur marked this pull request as draft April 8, 2024 14: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.

2 participants