Opened 12 years ago
Closed 8 years ago
#664 closed defect (fixed)
Overflow not handled at pages like delete media ones
Reported by: | pythonsnake | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | graphic design/interface | Keywords: | overflow css bitesized sprint |
Cc: | Jiyda Mint Moussa | Parent Tickets: |
Description
See the attached image
Attachments (3)
Change History (26)
by , 12 years ago
Attachment: | HrpxoGE.png added |
---|
comment:1 by , 12 years ago
Keywords: | bitesized sprint added; bitsized removed |
---|
by , 11 years ago
Attachment: | 664_delete_media_overflow.png added |
---|
comment:2 by , 11 years ago
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.
comment:3 by , 11 years ago
Owner: | removed |
---|---|
Status: | new → review |
Someone good at css should review that. Not me.
comment:4 by , 11 years ago
Owner: | set to |
---|---|
Status: | review → in_progress |
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?
comment:6 by , 11 years ago
Milestone: | 0.5.0 → 0.6.0 |
---|
comment:7 by , 11 years ago
Milestone: | 0.6.0 → 0.7.0 |
---|
comment:8 by , 10 years ago
Milestone: | 0.7.0 |
---|
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.
comment:9 by , 9 years ago
Owner: | removed |
---|---|
Status: | in_progress → accepted |
comment:10 by , 8 years ago
Cc: | added |
---|
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
comment:12 by , 8 years ago
comment:13 by , 8 years ago
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
by , 8 years ago
Attachment: | Screenshot from 2016-09-12 16:55:00.png added |
---|
Edit view still overflowing
comment:14 by , 8 years ago
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.
comment:15 by , 8 years ago
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
comment:16 by , 8 years ago
Thanks Kesra, I've verified that this works for me in both Edit and Delete views. Ready to be reviewed I think!
comment:17 by , 8 years ago
Status: | accepted → review |
---|
comment:18 by , 8 years ago
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.
- Rebase your branch against current mediagoblin master.
- 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.
- 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.)
- 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!
comment:19 by , 8 years ago
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.
comment:20 by , 8 years ago
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.
comment:21 by , 8 years ago
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?
comment:22 by , 8 years ago
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.
comment:23 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | review → closed |
Fixed title overflow in delete media page