Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Avoid the edit icon to move in the file exclusion set - #12671

Merged
marcelgerber merged 1 commit into
adobe:masterfrom
ficristo:fix-filter-pencil-pos
Aug 14, 2016
Merged

marcelgerber merged 1 commit into
adobe:masterfrom
ficristo:fix-filter-pencil-pos

Conversation

@ficristo

Copy link
Copy Markdown
Collaborator

This should fix or at least help with #10963

@marcelgerber

Copy link
Copy Markdown
Contributor

I suppose the empty line isn't supposed to show up any more? For me, it still does...
image

Also, there are the related issues #9305 and #10962. Maybe you can find a fix that handles all 3 of them?

@ficristo

Copy link
Copy Markdown
Collaborator Author

It seems to me you are resizing the window so it's #10962?
Unfortunately fixing #10962 is out for me, I fought enough against CSS with this...

@marcelgerber

Copy link
Copy Markdown
Contributor

No, the window is at full size and has been at this size the whole time.

@ficristo
ficristo force-pushed the fix-filter-pencil-pos branch from ac3b303 to b3107af Compare August 14, 2016 13:16
@ficristo

Copy link
Copy Markdown
Collaborator Author

With latest changes I should have fixed your issue too. But still not #10962.

Comment thread src/widgets/DropdownButton.js Outdated
posTop = Math.max(0, toggleOffset.top - $dropdown.height() - 4);
}

// Take in consideration the scrollbar to prevent the edit icon to move down. See #10963

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we shouldn't mention the edit icon here because DropdownButton is far more versatile.
I'd go for something along the lines of Take the scrollbar into consideration so we don't get unintented line breaks (see #10963)

@marcelgerber marcelgerber self-assigned this Aug 14, 2016
@marcelgerber

Copy link
Copy Markdown
Contributor

Thanks! LGTM now and handles both issues indeed :shipit:
Just that tiny thing about the comment and we're ready to go!


// Take in consideration the scrollbar to prevent the edit icon to move down. See #10963
var dropdownElement = this.$dropdown[0];
var scrollWidth = dropdownElement.offsetWidth - dropdownElement.clientWidth + 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, where does the + 1 come from? Is it just a safety measure so we have a tiny little more space available or does it serve a real purpose?

@ficristo
ficristo force-pushed the fix-filter-pencil-pos branch from b3107af to 762d2b7 Compare August 14, 2016 17:22
@ficristo

Copy link
Copy Markdown
Collaborator Author

Actually that 1 pixel is the fix!
Only after found it i decided to look at scrollbar to fix #10963 too.

@marcelgerber

Copy link
Copy Markdown
Contributor

Thank you!
Merging.

@ficristo

Copy link
Copy Markdown
Collaborator Author

Thank you for all the reviews.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants