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 size of plain text editor #67

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mattfullerton
Copy link
Contributor

Pages' markdown editor for plain text editing is calling the same form that CKAN uses for small things. But as pages tend to be longer, some more space would be nice. This PR creates a new CSS class and passes it through to make the textarea larger.

Copy link
Contributor

@torfsen torfsen 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 idea!

Does this also work for our other editors (medium / ckeditor)?

@@ -15,3 +14,6 @@
.page-list-item h3 a {
display: block;
}
.page-simple-form-height {
height: 400px !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of !important. Wouldn't it be better to target the form via its ID (#field-content) in addition with a custom class?

Also, page-simple-form-height as a name doesn't tell me much. I'd also prefer a pages prefix for namespacing. How about pages-text-editor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has nothing to do with the other editors. In fact there the pixel heights are set and for the plain text editor no height is set which is what I am trying to improve.

I think !important here is justified as we are overriding the normal CKAN behaviour as we are using this form in a weird way. It might have been/be better overall to not use this macro to create the form.

With page vs. pages - I am following the convention in the existing CSS. Simple-form-height seems a lot more precise to me vs. text-editor (see comment above).

Regarding the first commit, that is me pulling in the latest ckanext-pages changes, but in a rather stupid way via. the GitHub UI.

@torfsen
Copy link
Contributor

torfsen commented Oct 4, 2017

Also, what about that first commit (c16ca58) in this PR? According to GitHub you merged changes from the change log, but that's not what the commit message says and I'm not sure how this is related to this PR...

@torfsen
Copy link
Contributor

torfsen commented Jan 24, 2018

ping @mattfullerton

@mattfullerton
Copy link
Contributor Author

@torfsen I've actually answered all the points above in the comment - would be nice to keep discussion on such a minor change to a minimum

@torfsen torfsen mentioned this pull request Feb 5, 2018
@EricSoroos EricSoroos deleted the feature-bigger-plain-editor branch October 5, 2020 11:26
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.

2 participants