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

(Issue 3403) Fix iOs left hand nav sliding choppiness #50

Closed
wants to merge 7 commits into from
Closed

(Issue 3403) Fix iOs left hand nav sliding choppiness #50

wants to merge 7 commits into from

Conversation

timdegroote
Copy link

No description provided.

@@ -541,6 +541,8 @@ div.oae-thumbnail i[class^="icon-"] {
margin: -16px 10px 10px;
Copy link
Owner

Choose a reason for hiding this comment

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

Can you reorder the added CSS alphabetically?

@bertpareyn
Copy link
Owner

Works consistently across the devices I tested, only some minor comments remaining.

@timdegroote
Copy link
Author

Followed up on review. @BP323

@bertpareyn
Copy link
Owner

@timdegroote I found a functional issue with the share widget in the left hand navigation on IE9. it looks like it's not overflowing properly and get cuts off where the list ends.

@timdegroote
Copy link
Author

Committed a fix which also takes care of some other issues. @BP323

@bertpareyn bertpareyn self-assigned this Feb 3, 2014
@bertpareyn
Copy link
Owner

@timdegroote could you have a look at fixing the merge conflict?

…ue-3403

Conflicts:
	shared/oae/js/jquery-plugins/jquery.responsive.js

/**
* Open the left hand navigation
* Toggle the left hand navigation. Throttle the function to prevent it from being triggered during animation.
* @param {Boolean} visible Determines whether the left hand navigation should be shown (true) or hidden (false).
Copy link
Owner

Choose a reason for hiding this comment

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

It'd make more sense to rename this to showNav I think.

@bertpareyn
Copy link
Owner

Some comments remaining, reassigning to @timdegroote for followup

@timdegroote
Copy link
Author

Follow-up done @BP323

@bertpareyn bertpareyn self-assigned this Feb 5, 2014
@bertpareyn
Copy link
Owner

Functionality is working as expected and code looks good, just a few minor doc updates suggested.

@timdegroote
Copy link
Author

Submitted a follow-up @BP323

@bertpareyn
Copy link
Owner

@timdegroote can you fix the merge conflict

…ue-3403

Conflicts:
	shared/oae/css/oae.components.css
@timdegroote
Copy link
Author

Merge conflict should be fixed @BP323

@bertpareyn
Copy link
Owner

Looks good, ready to merge.

@bertpareyn bertpareyn removed their assignment Feb 11, 2014
@bertpareyn bertpareyn closed this Feb 13, 2014
@bertpareyn
Copy link
Owner

Closing in favor of oaeproject#3522

bertpareyn pushed a commit that referenced this pull request Sep 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants