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

Alerts stretch images #10

Open
dhornbein opened this issue May 24, 2017 · 15 comments
Open

Alerts stretch images #10

dhornbein opened this issue May 24, 2017 · 15 comments
Assignees
Labels

Comments

@dhornbein
Copy link

.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

@dhornbein dhornbein self-assigned this May 24, 2017
@dhornbein dhornbein added the bug label May 24, 2017
@flavour
Copy link
Member

flavour commented May 24, 2017

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
Copy link
Author

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

  • Limit Alert stubs to 300px height.
  • Limit Event stubs to 500px height

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
Copy link
Member

flavour commented Jun 30, 2017

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
Copy link
Author

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
Copy link
Member

flavour commented Jun 30, 2017

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

@devinbalkind
Copy link
Member

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

@dhornbein
Copy link
Author

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
Copy link
Author

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
Copy link
Member

flavour commented Jul 14, 2017

See my 1st comment on this thread

@dhornbein
Copy link
Author

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

@flavour
Copy link
Member

flavour commented Jul 14, 2017

Sounds right, if you can :)

@dhornbein
Copy link
Author

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
Copy link
Member

flavour commented Jul 14, 2017

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
Copy link
Author

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
Copy link
Member

flavour commented Jul 17, 2017

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

  • Videodetector
  • Widget

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants