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

remove flex causing Firefox text area height issue, fix #1943 #2088

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@vanesa
Copy link
Member

vanesa commented Jan 31, 2019

Issue: The Monaco editor in the user settings page was shown very small in Firefox.

Solution: Remove flex:1 set in the SettingsFile.scss which was being applied to the Monaco editor text area but doesn't seem to be necessary.

Fixes #1943

@vanesa vanesa requested a review from felixfbecker as a code owner Jan 31, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 31, 2019

Codecov Report

Merging #2088 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted Files Coverage Δ
pkg/search/backend/backend.go 93.22% <0%> (-1.7%) ⬇️

.monaco-editor-container {
flex: 1;
}

This comment has been minimized.

@felixfbecker

felixfbecker Jan 31, 2019

Member

This was probably there so the editor adapts to the available space. Did you verify it still does that?

This comment has been minimized.

@vanesa

vanesa Jan 31, 2019

Author Member

When changing the screen size, the editor adapts correctly. I'm even wondering if we need

.settings-file {
    flex: 1;
}

at all, as the editor also adapts correctly without that code.

This comment has been minimized.

@vanesa

vanesa Jan 31, 2019

Author Member

image

Maybe this was an old workaround that is no longer necessary?

This comment has been minimized.

@vanesa

vanesa Feb 1, 2019

Author Member

Actually I think removing d-flex is the better approach.

@vanesa vanesa self-assigned this Feb 1, 2019

@vanesa vanesa requested a review from felixfbecker Feb 1, 2019

@felixfbecker

This comment has been minimized.

Copy link
Member

felixfbecker commented Feb 1, 2019

Without d-flex, how would the editor still adapt to screen size? Are you 100% confident it still does that?

@vanesa

This comment has been minimized.

Copy link
Member Author

vanesa commented Feb 1, 2019

From what I've tested in Firefox and Chrome and Safari: Yes
d-flex just contained flex: 1, which settings-file contains too.
image

@felixfbecker

This comment has been minimized.

Copy link
Member

felixfbecker commented Feb 1, 2019

Are you sure it is resizing correctly? If I zoom in on that screenshot I don't see the bottom border of the editor, which would hint that it actually overflows

@vanesa

This comment has been minimized.

Copy link
Member Author

vanesa commented Feb 1, 2019

You have to scroll down to see the bottom:
image

@felixfbecker

This comment has been minimized.

Copy link
Member

felixfbecker commented Feb 1, 2019

The goal of the flexbox styling is so that the editor grows and shrinks to fill the available space, instead of having a fixed height and forcing the user to scroll down

@vanesa

This comment has been minimized.

Copy link
Member Author

vanesa commented Feb 1, 2019

Yes. The 400px is to have the text area a certain height for when there is no content. We can reduce to to 250px if 400px seems too much.

@vanesa vanesa force-pushed the vo/monaco-firefox-settings branch from 4479861 to 4c7cac4 Feb 8, 2019

@felixfbecker

This comment has been minimized.

Copy link
Member

felixfbecker commented Feb 13, 2019

The editor should grow to fill the available space:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment