Skip to content

Fix mweb console #2640

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

Merged

Conversation

PiyushChandra17
Copy link
Contributor

Fixes #2631

Changes:

  • Added the missing props on SplitPane component in mobile version
  • Added stop button above console
fix-mweb-console.mov

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

Copy link
Collaborator

@lindapaiste lindapaiste left a comment

Choose a reason for hiding this comment

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

Looking good. In the existing code we have a gap below the button which we need to maintain. It is the bottom: ${remSize(20)}; which we are overwriting. We could add 20 to the offset from props. We could also change the bottom: ${remSize(20)}; to margin-bottom: ${remSize(20)}; so that it's a separate property which doesn't get overwritten.

I'm also noticing that dragging the console is extremely sluggish when I shrink my browser window to mobile size. I wonder why that is and I want to investigate, but I think it's beyond the scope of this PR.

When you collapse the console the button is too high. We need additional logic in the IDEView. Let me know if you need help with that. I think we probably want to extract the ide.consoleIsExpanded ? consoleSize : 29 part to a variable and use that variable for the offset instead of consoleSize.

@PiyushChandra17
Copy link
Contributor Author

@lindapaiste Yes i agree it's quite sluggish when it's narrowed down to mobile size. I think we should make a seperate issue for this and fix this problem in another PR. And as far as maintaining the gap below the button is concerned, i am looking at this and probably fix this soon. Thanks!

Copy link
Collaborator

@lindapaiste lindapaiste left a comment

Choose a reason for hiding this comment

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

Looking good! The sluggishness was fixed in another PR so I merged that in. I also fixed the offsetBottom (and assigned an intermediate variable currentConsoleSize so that eslint wouldn't complain about nesting ternary operators).

@lindapaiste lindapaiste merged commit 18c910a into processing:develop Jan 6, 2024
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.

Clicking on down chevron beside clear does not shifts the console to bottom in mWeb(Safari) && mWeb(chrome)
2 participants