sahana / WACOP

Washington State Common Operating Picture
0 stars 0 forks source link

Alerts stretch images #10

Open dhornbein opened 7 years ago

dhornbein commented 7 years ago

.alert-card elements (and others I would imagine) with images are stretching those images. The image size is set as an inline style thus can't be overwritten without !important flags, which would be best to avoid.

screen shot 2017-05-24 at 10 16 05 am

flavour commented 7 years ago

Note that I think the inline style is coming from the WYSIWYG editor & not from Sahana. We include the source for CKEditor here: https://github.com/sahana/eden/tree/master/static/ckeditor Currently using 4.5.6...4.6.2 is now available & they /may/ have addressed this issue there so probably worth an upgrade as even if we need to hotfix then we should do that on the latest version, with ideally a Pull Request to the upstream project.

dhornbein commented 7 years ago

Devin has requested these "stubs" be limited in height. How is best to implement the following:

I can set a max-height to the body of the box, which would could cut off the content with CSS overflow: hidden. However there isn't any indication that texts is being cut off and there is no way to add style only if content is being cut off.

Another option would be to set a max-height and overflow-y:scroll which would set a max height but allow users to scroll through all the content without having to click "read more"

Finally we could do something with javascript.

Perhaps @flavour you can sip them on the controller side? Then adding a class we could style them.

Thoughts?

flavour commented 7 years ago

I'm not sure how we can snip server-side to a certain # of px, but we can certainly snip & experiment with how much...can you give a suggestion for how much to snip? Of course with embedded pictures this becomes even more complicated :/ What is the'something' that can be done with JS? JS certainly shouldn't be seen as a blocker if there is a workable approach

dhornbein commented 7 years ago

JS could measure the hight can add a class that would hide the overflow and set a style that would indicate that.

I think the word count probably isn't going to work due to images (and other rich text styling).

I'll see what I can do with JS.

flavour commented 7 years ago

Agree that JS approach seems best...this needs to be client-side.

devinbalkind commented 7 years ago

Can we strip images out in the stubs? Does that enable us to solve this problem?

dhornbein commented 7 years ago

I've come up with a CSS based solution that also allows us to maintain consistent height for all alert and event elements. Once it's up on the server we should test then close the issue.

dhornbein commented 7 years ago

There is still the issues that images are getting squished!

Adding css

.card .desc {
  max-width: 100% !important;
  height: auto;
}

would fix the issue, however I'd rather not do this! !Important tags are evil.

@flavour is there any way to prevent the system from applying width and height attributes to the element?

flavour commented 7 years ago

See my 1st comment on this thread

dhornbein commented 7 years ago

whoops, yeah. What's the directive here? Should I update the editor, test it, and make a pull request?

flavour commented 7 years ago

Sounds right, if you can :)

dhornbein commented 7 years ago

I've updated to 4.7.1, simply replaced all files apart from /static/ckeditor/config.js, appears to work as expected but the editor bar doesn't contain the same buttons.

current: screen shot 2017-07-14 at 2 48 54 pm

new version: screen shot 2017-07-14 at 2 48 59 pm

Is there another place where this is configured?

Also, the download page has options to download basic, standard, and full. I've tried both basic and standard, do you know which we use?

flavour commented 7 years ago

I guess we use basic + necessary plugins (check the plugins folder...we have 15 currently) This is where we configure the toolbar: https://github.com/sahana/eden/blob/master/modules/s3/s3widgets.py#L8258

I'm not sure if the API changed at all from 4.5.x to 4.7.x?

dhornbein commented 7 years ago

Nope, syntax appears to be the same and valid.

I've pushed the updated code into my eden repo under a new branch if you want to check it out: https://github.com/dhornbein/eden/tree/ckeditor

flavour commented 7 years ago

I did the upgrade...turned out to be the 'Standard' install + these 2 plugins:

So now you need to see if the problem is still there (I would guess it is) & then figure out where in the code the change should be made. Can then be hacked into place - with clear comments. & a clean patch submitted upstream: https://github.com/ckeditor/ckeditor-dev