Overflow not handled at pages like delete media ones

[pythonsnake]

See the attached image

[Jiyda Mint Moussa]

Fixed title overflow in delete media page

[Jiyda Mint Moussa]

I fixed this issue by modifying base.css here
​https://gitorious.org/~jiyda/mediagoblin/jiydas-mediagoblin/commit/a4164ea3029d12b94c81389f265afb791131b3f3
Attached is a screenshot of the result.

[Elrond]

Someone good at css should review that. Not me.

[joar]

The commit submitted by jiyda included a lot of ltr changes, besides that, the fix looks great! Just as seen in the screenshot provided :)

Jiyda, could you please separate the overflow fix from the LTR-stuff and resubmit it here?

[Christopher Allan Webber]

Moving to 0.4.1

[Christopher Allan Webber]

Is this still true? We had a CSS overhaul when Skeleton was merged. It would be good for someone to check.

Regardless, I'm removing the milestone.

[Ben Sturmfels]

Hi Jiyda,

It looks as though the Gitorious branch mentioned has now been lost. Can you recall how you made this change? I wasn't aware that you could force browsers to wrap long strings like "oooooooooooooooooooooooooooooooooo". The best option I can see is to set overflow: hidden; text-overflow: ellipsis. Not sure how to replicate your second screenshot above.

(Confirming that this is still an issue as of 0.9.0)

Cheers,
Ben

[Kesara]

This issue exits on both delete view and edit view.

[Kesara]

Bug fix: ​https://github.com/kesara/mediagoblin/commit/8368c7373db76a8f530be0b15be55148642a3939

[Ben Sturmfels]

Hi Kesra,

Thanks for spending time sprinting with us today!

I've just checked out your branch bugfix-664. The change looks great on the "delete" view. The "edit" view must be a little different though, as this appears unchanged. Must be different CSS classes.

Could we also apply the change to the main "viewing a single item" view?

Thanks again,
Ben

[Ben Sturmfels]

Edit view still overflowing

[Ben Sturmfels]

Actually, perhaps don't look at the "single item view" after all - I think this is less of an issue since it spans the full screen width, unlike the "edit" and "delete" pages.

[Kesara]

Ah, I was meant to add .form_box_xl CSS class as well.
I have the changes on: ​https://github.com/kesara/mediagoblin/commits/bugfix-664

[Ben Sturmfels]

Thanks Kesra, I've verified that this works for me in both Edit and Delete views. Ready to be reviewed I think!

[ayleph]

Hi kesara,

It's difficult to review your code on a foreign branch. Here's what you can do to help us review your code for merging.

1. Rebase your branch against current mediagoblin master.

2. Use the interactive rebase command to squash your commits and provide a single, clean commit which contains all of the changes. It's okay to use multiple commits if you have multiple independent changes, but if they're all related, then one commit is preferred.

3. Prepare a patch of your differences from master using the git format-patch command. Make sure that your git environment includes a name and email address that you're okay with being included in the mediagoblin commit history. Check that this is correct in the patch file created with git format-patch. (If you don't want your name or email address included in the mediagoblin commit history, remove them from the patch file or let us know.)

4. Upload your patch file as an attachment to this issue.

I know it's a lot of extra work for you, but the easier you make things for the developers, the more likely your patch is to be applied. Thanks for your work!

[Ben Sturmfels]

Hi Ayleph,

Sorry, that's my fault for recommending that kesara use a remote git branch. The wiki does suggest remote branches as "the preferred method of sending changes" though:

​https://wiki.mediagoblin.org/Git_workflow#How_to_send_us_your_changes

If that's no longer the preferred approach, could you please update the wiki? I don't feel like I'm qualified to make this change on behalf of the project.

With regards to squashing - once you go and squash it, reviewers can no longer simply pull to receive your subsequent changes, making it hard to collaborate on change. I'd suggest not squashing and instead using git diff ...origin/master when reviewing.

[ayleph]

Hi Sturm,

You're not wrong in either of your points. Regarding the text on the wiki, perhaps a remote branch is still the preferred method for some. Personally, I find it easier to test a single patch file provided here against my local copy of current master than to check someone's remote branch for changes against whatever version of master they happen to have. Having someone provide a git format-patch style patch also makes it very easy for me to sign off on the change and apply it directly, instead of trying to copy their branch's changes over to my instance.

With regards to squashing changes, my counterpoint is that sometimes a line of code gets changed multiple times in multiple commits, which makes it hard to review when looking at it piece-wise. As an example, the commits made on this ticket's remote branch end up changing one line of code three separate times. It would be easier to follow if all of those were squashed into a single change that provided the end result, instead of having multiple commits that changed the same bit of code. Maybe we can compromise and say that multiple commits are useful in the interim for collaboration, but when it comes time to merge, squashing into fewer atomic changes is better.

[ayleph]

I've pulled the changes from kesara's remote branch into a local instance for testing, and it looks good. I'm wondering if we should apply similar changes to the following.

• The blog_form_box_xl class
• The various classes in the airy theme

Does that seem like something that should be part of this ticket as well?

[Ben Sturmfels]

Hi Ayleph,

I'd suggest that we keep the focus fairly narrow here and deal separately with any other overflow problems when they are reported. That's partly because I'd like to see Kesara's first patch not get held up, and partly because, if we start looking hard, ugly overflows can crop up in plenty of places.

The blog media type docs say "highly experimental" and "not for production", so I suggest we don't let that particular CSS class keep this ticket open.

PS. Thanks for taking the time to review. Your points about single patch files and squashing make total sense. I'd certainly also support a squash at time of merge on a case-by-case basis.

[ayleph]

kesara,

Thank you for your contributions. Your changes have been applied in ​7ee8e6b. You have been added to the list of AUTHORS in ​c7c3eac. If you would prefer to be recognized under a different name, please let us know, and we'll update the list.