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

Limit propagation of new part name/link for #2917 #2918

Merged
merged 2 commits into from Mar 11, 2015

Conversation

anitagraham
Copy link
Contributor

Between creation of new page part and save of page with new part, the
other editor tabs would have the link to the new page part added to
their content area.

This fix ensures the link is only added to the page tabs.

Between creation of new page part and save of page with new part, the
other editor tabs would have the link to the new page part added to
their content area.

This fix ensures the link is only added to the page tabs.
@parndt
Copy link
Member

parndt commented Mar 8, 2015

Hmm, this seems like it's supposed to be passed in. Can we limit the scope of the function to just apply to the one it's supposed to? I don't want to break this for other uses accidentally

@anitagraham
Copy link
Contributor Author

Do you mean like this:

page_options.tabs.find("ul#page_parts").append(
  "<li><a href='#page_part_new_"+$('#new_page_part_index').val()+"'>"+part_title+"</a></li>"
);

@parndt
Copy link
Member

parndt commented Mar 9, 2015

Yup, exactly.

@anitagraham
Copy link
Contributor Author

OK... but as I'm targeting an id the effect should be identical.

@parndt
Copy link
Member

parndt commented Mar 9, 2015

Until you have two of these on the same page 😄

@anitagraham
Copy link
Contributor Author

  1. Aren't there are always just one set of page tabs on a page?
  2. If it is an id there should only be one on the page. (Two identical ids will work most of the time, but not always)
    I am changing it, btw, just arguing for the sake of it because I think I'm right. 😜

@parndt
Copy link
Member

parndt commented Mar 9, 2015 via email

@anitagraham
Copy link
Contributor Author

I'd like to see an example of a container/page with two sets of tabs for content sections.

@anitagraham
Copy link
Contributor Author

Travis fail....whyowhyowhy.
<aside>
If you look at the generated HTML for refinery there are (lots, some, a few) occurrences of tags which have identical class and id attributes. If one were scrubbing Refinery down then these would be candidates for a good cleanup. And then the CSS could be cleaned up too....
</aside>

parndt added a commit that referenced this pull request Mar 11, 2015
Limit propagation of new part name/link for  #2917
@parndt parndt merged commit a0d8052 into refinery:master Mar 11, 2015
@anitagraham anitagraham deleted the ppNamePropagation branch March 11, 2015 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants