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

Increase the width of string editor modal #1140

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

Georift
Copy link
Contributor

@Georift Georift commented Oct 13, 2018

Overview

When I first used Fauxton I found the default width of the text editor modal (560px) to be a little small. If you were to view the users validate_doc_update function there would always be an overflow.

This PR increases the width of the modal by default and removed the padding surrounding the editor to give it a cleaner look.

Testing recommendations

This change could potentially affect some of the other uses of the modals.

GitHub issue number

No issue number

Checklist

  • Code is written and works correctly;

Copy link
Contributor

@Antonio-Maranhao Antonio-Maranhao left a comment

Choose a reason for hiding this comment

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

I like the new look. Just need a small change (see below).
Also, can you please remove package-lock.json since there were no changes to packages.json ?

@@ -25,9 +25,8 @@
position: fixed;
top: 10%;
left: 50%;
transform: translateX(-50%);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather see these overrides in fauxton.less. There's already a section with some in there (search for bootstrap overrides).
I was able to make it work by adding this to fauxton.less:

.modal {
  transform: translateX(-50%);
  margin-left: inherit;
}
.modal-dialog {
  display: flex;
  flex-direction: row;
}
 .modal-content {
  flex: 1;
  width: 560px;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry, I didn't realise those were bootstrap stylings. I've moved them like you suggested.

@Georift Georift force-pushed the doc-editor-ui-improvements branch 2 times, most recently from 95d60ac to 24a5d94 Compare October 21, 2018 06:19
@Georift
Copy link
Contributor Author

Georift commented Oct 21, 2018

Addressed the changes and removed the package-lock.json. Thanks for taking the time. 😄

Moves specified width styling from the parent .modal the .model-content
so child models are able to specify their own width. Also removed the
modal padding surrounding the text editor.

Fixed an issue with mouse pointer not being visible on a few buttons.
It was using `cursor: hand` which was supported only supported in very
old IE's.
@Georift
Copy link
Contributor Author

Georift commented Oct 23, 2018

Rebased on master, no code changes.

@Georift
Copy link
Contributor Author

Georift commented Oct 24, 2018

Hey @Antonio-Maranhao, is there anything else you'd like changed?

@Antonio-Maranhao
Copy link
Contributor

Hey @Georift not really. Just didn't have time to take it for a final spin. I'll get to it in a bit.

@Antonio-Maranhao Antonio-Maranhao merged commit e8f9600 into apache:master Oct 24, 2018
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.

None yet

2 participants