Overqualified Selectors #700

Closed
PirataNervo opened this Issue Jun 3, 2014 · 18 comments

Comments

Projects
None yet
6 participants
@PirataNervo
Contributor

PirataNervo commented Jun 3, 2014

Fix CSS selectors. (see original thread for list - I'll post it here when I work on this)

Original thread: Overqualified Selectors

@PirataNervo PirataNervo added this to the 1.8 Beta 2 milestone Jun 3, 2014

@PirataNervo PirataNervo added 1.8 and removed bug labels Jun 3, 2014

@Eric-Jackson

This comment has been minimized.

Show comment
Hide comment
@Eric-Jackson

Eric-Jackson Jun 3, 2014

Contributor

I want to attempt to work on this, but I'm new to git so if I do something wrong my bad. 💃

Contributor

Eric-Jackson commented Jun 3, 2014

I want to attempt to work on this, but I'm new to git so if I do something wrong my bad. 💃

@Eric-Jackson

This comment has been minimized.

Show comment
Hide comment
@Eric-Jackson

Eric-Jackson Jun 3, 2014

Contributor

Well those are taken care of, I'll try and look into the others.

Contributor

Eric-Jackson commented Jun 3, 2014

Well those are taken care of, I'll try and look into the others.

@Eric-Jackson

This comment has been minimized.

Show comment
Hide comment
@Eric-Jackson

Eric-Jackson Jun 4, 2014

Contributor

Just looking through these files, inline_moderation.js and the related controls in templates are extremely strict, which really limits the theme developers. I don't know github/mybb guidelines well enough to know if I should attempt a fix at that here, or start another branch/issue to keep them separate (It's still relating to selectors and lack of classes). Or if I should fix it at all for that matter.

Contributor

Eric-Jackson commented Jun 4, 2014

Just looking through these files, inline_moderation.js and the related controls in templates are extremely strict, which really limits the theme developers. I don't know github/mybb guidelines well enough to know if I should attempt a fix at that here, or start another branch/issue to keep them separate (It's still relating to selectors and lack of classes). Or if I should fix it at all for that matter.

@PirataNervo

This comment has been minimized.

Show comment
Hide comment
@PirataNervo

PirataNervo Jun 26, 2014

Contributor

I'll go through these soon and try to get them into MyBB 1.8 Beta 2. After we can work on improving the JS files.

Contributor

PirataNervo commented Jun 26, 2014

I'll go through these soon and try to get them into MyBB 1.8 Beta 2. After we can work on improving the JS files.

@PirataNervo PirataNervo self-assigned this Jun 26, 2014

PirataNervo added a commit that referenced this issue Jun 28, 2014

Merge pull request #707 from ATofighi/wip-700
Working on #700 - Overqualified Selectors
@PirataNervo

This comment has been minimized.

Show comment
Hide comment
@PirataNervo

PirataNervo Jun 28, 2014

Contributor

@Eric-Jackson can you please update your PR? There are conflicts and I can't merge it. I've also commented on a line there which I believe you made a mistake.

Contributor

PirataNervo commented Jun 28, 2014

@Eric-Jackson can you please update your PR? There are conflicts and I can't merge it. I've also commented on a line there which I believe you made a mistake.

@Eric-Jackson

This comment has been minimized.

Show comment
Hide comment
@Eric-Jackson

Eric-Jackson Jun 28, 2014

Contributor

@PirataNervo I don't know how to update it yet, I'll try to figure it out though. But there's no error, the code is still there. I've only changed the selector.

Contributor

Eric-Jackson commented Jun 28, 2014

@PirataNervo I don't know how to update it yet, I'll try to figure it out though. But there's no error, the code is still there. I've only changed the selector.

@PirataNervo

This comment has been minimized.

Show comment
Hide comment
@PirataNervo

PirataNervo Jun 28, 2014

Contributor

@Eric-Jackson conflicts mean that the PR cannot be merged automatically because other changes have been made meanwhile and the difference generated bit GH at the time you submitted your PR can no longer be applied. You either resolve the conflicts yourself or (if you don't know how to do it) you submit a new PR by creating the changes again.

Contributor

PirataNervo commented Jun 28, 2014

@Eric-Jackson conflicts mean that the PR cannot be merged automatically because other changes have been made meanwhile and the difference generated bit GH at the time you submitted your PR can no longer be applied. You either resolve the conflicts yourself or (if you don't know how to do it) you submit a new PR by creating the changes again.

@Eric-Jackson

This comment has been minimized.

Show comment
Hide comment
@Eric-Jackson

Eric-Jackson Jun 28, 2014

Contributor

@PirataNervo Alright I've created a new branch and pull request.

Contributor

Eric-Jackson commented Jun 28, 2014

@PirataNervo Alright I've created a new branch and pull request.

@PirataNervo

This comment has been minimized.

Show comment
Hide comment
@PirataNervo

PirataNervo Jun 28, 2014

Contributor

Thanks, I've commented on a line.

Contributor

PirataNervo commented Jun 28, 2014

Thanks, I've commented on a line.

@Eric-Jackson

This comment has been minimized.

Show comment
Hide comment
@Eric-Jackson

Eric-Jackson Jun 28, 2014

Contributor

The elements were .each() looped twice, once for div.trow_selected and once for td.trow_selected. Both times removing .trow_selected. By simplifying those selectors, the loops become the same code, so I removed one block of it.

Contributor

Eric-Jackson commented Jun 28, 2014

The elements were .each() looped twice, once for div.trow_selected and once for td.trow_selected. Both times removing .trow_selected. By simplifying those selectors, the loops become the same code, so I removed one block of it.

@PirataNervo

This comment has been minimized.

Show comment
Hide comment
@PirataNervo

PirataNervo Jun 28, 2014

Contributor

Alright, removed my comment. Apparently I missed the lines just below that. Thank you! Going to test it now.

Contributor

PirataNervo commented Jun 28, 2014

Alright, removed my comment. Apparently I missed the lines just below that. Thank you! Going to test it now.

@PirataNervo

This comment has been minimized.

Show comment
Hide comment
@PirataNervo

PirataNervo Jun 28, 2014

Contributor

Merged! I'm closing this now. Or are there any others you'd like to work on?

Contributor

PirataNervo commented Jun 28, 2014

Merged! I'm closing this now. Or are there any others you'd like to work on?

@PirataNervo PirataNervo added the fixed label Jun 28, 2014

@Eric-Jackson

This comment has been minimized.

Show comment
Hide comment
@Eric-Jackson

Eric-Jackson Jun 28, 2014

Contributor

There are some more things I'd like to work on, but they aren't "Overqualified selectors", just "Strict element targeting". Maybe that would be best in another issue.

Contributor

Eric-Jackson commented Jun 28, 2014

There are some more things I'd like to work on, but they aren't "Overqualified selectors", just "Strict element targeting". Maybe that would be best in another issue.

@PirataNervo

This comment has been minimized.

Show comment
Hide comment
@PirataNervo

PirataNervo Jun 28, 2014

Contributor

Alright, closing this. Feel free to open a new issue on the forums :)

Contributor

PirataNervo commented Jun 28, 2014

Alright, closing this. Feel free to open a new issue on the forums :)

@Stefan-ST

This comment has been minimized.

Show comment
Hide comment
@Stefan-ST

Stefan-ST Aug 23, 2014

Contributor

These changes introduced invalid HTML:

document type does not allow element "div" here; missing one of "object", "applet", "map", "iframe", "button", "ins", "del" start-tag

<dt><div class="forum_status forum_on" title="Forum Contains New Posts"></div></dt>
Contributor

Stefan-ST commented Aug 23, 2014

These changes introduced invalid HTML:

document type does not allow element "div" here; missing one of "object", "applet", "map", "iframe", "button", "ins", "del" start-tag

<dt><div class="forum_status forum_on" title="Forum Contains New Posts"></div></dt>

@Stefan-ST Stefan-ST reopened this Aug 23, 2014

@Stefan-ST Stefan-ST modified the milestones: 1.8 Beta 2, 1.8.0 Aug 23, 2014

@Stefan-ST Stefan-ST added s:feedback and removed s:fixed labels Aug 23, 2014

@Destroy666x

This comment has been minimized.

Show comment
Hide comment
@Destroy666x

Destroy666x Aug 23, 2014

Contributor

Changing the forum legend <div>s to <span>s with style="display: inline-block;" should fix it.

Contributor

Destroy666x commented Aug 23, 2014

Changing the forum legend <div>s to <span>s with style="display: inline-block;" should fix it.

ATofighi added a commit to ATofighi/mybb_ that referenced this issue Aug 23, 2014

@ATofighi

This comment has been minimized.

Show comment
Hide comment
@ATofighi

ATofighi Aug 23, 2014

Contributor

Fixed in #1299

Contributor

ATofighi commented Aug 23, 2014

Fixed in #1299

Sama34 added a commit that referenced this issue Aug 23, 2014

@Sama34

This comment has been minimized.

Show comment
Hide comment
@Sama34

Sama34 Aug 23, 2014

Contributor

Seems fixed to me.

Contributor

Sama34 commented Aug 23, 2014

Seems fixed to me.

@Sama34 Sama34 closed this Aug 23, 2014

@PirataNervo PirataNervo removed their assignment Jan 12, 2015

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