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

[Reblog Display Options] Update for React #2007

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dlmarquis
Copy link

Makes Reblog Display Options usable again. All features are implemented except the "post title on top" option in flat mode, and the padding options and colored background option in nested mode.

I think I've covered every preferences permutation, but there may be some I've missed.

Be aware, timestamps needed a minor update to make it compatible with nested mode.

data.reblog_author + "</a>:</p>" +
'<blockquote class="reblog-quote">' + all_quotes_text + reblog_content + "</blockquote>";
});
for (j = 0; j+1 < $(reblog_tree).get().length; j++) {
Copy link

Choose a reason for hiding this comment

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

missing var

if (this.preferences.remove_last_user.value) {
XKit.tools.add_css(".reblog-list-item.contributed-content .reblog-header {display: none;}", "better_reblogs");
list_sel = ".reblog-list ";
XKit.tools.add_css("div[data-is-contributed-content]>" + reblogHeader + " {display: none;}", "better_reblogs");
Copy link
Member

Choose a reason for hiding this comment

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

keyToCss returns a comma separated list of classes, so this will turn into the css string

div[data-is-contributed-content] > .class1, .class2 {
  display: none;
}

Instead, you'll want to do something like:

XKit.css_map.keyToClasses("reblogHeader", cls => `[data-is-contributed-content] > ${cls}`).join(", ")

Copy link
Author

Choose a reason for hiding this comment

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

I did forget to do this in a couple cases, but reblogHeader and others only return a single class. As far as I can tell, the number of list items is constant. Is it our convention to do it anyway?

Copy link

Choose a reason for hiding this comment

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

Yes, it's good to be future-proofed

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that while the list of classes is constant within a single release, Tumblr could add a new class named reblogHeader at any given time, and depending on compilation order/etc, that could massively break things. Just seems like it's safer to err on the side of caution here.

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.

3 participants