In some cases, #535 caused comment display to wildly break visible
page formatting. The sidebar often moved to bottom of page, and some
comments would display in a second column alongside the main one.
Root cause was a closing `</div>` left outside of a conditional
which was included in rendered markup even when:
1) A comment is invisible for the current browsing user v, e.g.
filtered or removed.
2) The subtree rooted at said comment is to be hidden based on
the display logic added in #535.
Which left a stray `</div>` with no opening tag where those
comments should've gone.
Most of the testing appears to have been done on reply chains of
comments, which behave correctly. It is specifically the zero
children (or otherwise not even showing a "Removed" message in the
page) which causes the bug.
* invisibleify completely removed trees only (fixes#431)
* fix visibility state for shadowbanned users.
this also ends up moving some of the complexity out of the templates.
* comments: remove unused variable
* moderation state machine
* no seriously this really should check for v not being None
* fix shadowban state
* fix visibility state
* update stateful counters
* don't use bespoke function for show_descendants
* properly mock ModerationState for cron submissions
* fix approval discrepency
* remove treenukes for removed comments
* show shadowbans as removed
* remove holes
* don't import class that doesn't exist
* remove subs in template code
* remove subs_created
* fix sticky bug
* uh... i think this will fix it????
* fix stupid nearly ungrokkable nesting error
* ...
* ....
* ....
* *sigh*
* .............................i think i got it.
* Revert ".............................i think i got it."
This reverts commit 419a545875.
* Revert "*sigh*"
This reverts commit fcfc9d7995.
* Revert "...."
This reverts commit e200c8f6f0.
* Revert "...."
This reverts commit 681db8cb02.
* Revert "..."
This reverts commit c54372b9ff.
* Revert "fix stupid nearly ungrokkable nesting error"
This reverts commit e202fd774c.
* Revert "uh... i think this will fix it????"
This reverts commit e4d9366093.
* readd missing endif
* fix email templates.
* fix frontpage listing
* some minor fixes wrt saving
* fix some listing errors
* Remove more references to holes system
A couple of these came from the merge of #554. A few others were just
left in the templates and presumably hard to catch on the first
removal pass.
* remove unnecessary lazy
* Add migration
Tested. Very sensitive to order. I wound up borrowing the statement
order we used for deleting sub rows on upstream (manually, of course).
---------
Co-authored-by: TLSM <duolsm@outlook.com>
changed in it
the comments.html template (along with submission.html) has numerous
undesirable properties which i will describe now. unless you are very
familiar with the codebase, it can be extremely difficult to grok.
this is pretty insane as there is nothing fundamentally complex about
the goal of comments.html: return a component that shows a username
and info, reports if any, comment content, and actions a user can
take.
this behemeoth was initially 886 lines in the old version of this
codebase, and this is with awards and a lot of other cruft removed.
anyway, the maintainability of this file is about on par with some
legacy application that keels over and dies if you sneeze vaguely
in its direction.
the nicest thing i can say about it is that it isn't currently
crashing.
anyway some of the problems include:
* large, splittable components, are not split into separate files.
this makes it incredibly difficult to find or make changes across
the template and makes it nearly impossible to find or change a
specific thing.
this is most easily exemplified in the modals, which should by all
accounts be separate templates, just inlined into comments.html.
* the nesting is oftentimes incorrect.
inexplicably, probably out of laziness from when the code was first
written, things will end up fully left aligned, while multiple layers
deep into a nesting context.
if an if statement or an endif is changed, it is *incredibly*
difficult to figure out where the error was. you can't trust the
nesting.
* multiple repeated checks for things that are always true.
this is probably a symptom of the above two problems but it's very
noticeable once you fix the nesting. for example there is a block
near the very top of the actions bar which checks for
parent_submission which effectively checks "is this in a post" (this
commit won't complain about parent_submission checks but i do have
opinions on those).
all of the action buttons further down the chain also check for
parent_submission, or even check inconsistently (by using if c.post)
within this context this is a completely unnecessary check in this
context.
while it is potentially useful (and in fact because #251 requires we
dismantle the assumption a little bit) to have these checks now, the
fact that they were initially added shows that when the code was all
initial written, there was little care into thinking about comment
state.
* mobile actions are duplicated and duplicated inline.
i actually do find it probably pretty hard to support this normally
given the codebase's DOM so whatever, duplicate the things, *but* if
we're going to do that, inlining it into the middle of an incredibly
long template is really difficult to comprehend as a design decision.
...anyway yeah this PR intends to fix these problems and enable work
to be done on #251. this is a "perfect is the enemy of good" commit.
it doesn't change much fundamental and is not intended to erase the
sins of the original file, but at least make it maintainable.
this also fixes a minor bug with #473 where the GIF modal was left
in by accident.
the random c.sentto == 2 magic numbers in the code is... pretty
unmaintainable and unless you were aware of who "2" was, it's hard to
know what's going on.
in addition, we force modmail to go through the modmail path instead of
letting users bypass validation checks.
these functions i don't believe exist upstream and i guess were to
diagnose an issue with author_ids not being saved properly. anyway not
really useful now as if an author_id doesn't exist, some major db
corruption has prolly already happened.
Removes the following awards / fields on User:
- flairlock
- progressivestack
- bird
- longpost (pizzashill)
- marseyawarded
- rehab
- deflector
- mute
- unmutable
- eye (All-Seeing Eye)
- alt (Alt-Seeing Eye)
Primarily motivated by starting to remove some un-Mottelike cruft
from core commenting/posting routes. Cleared out other inapplicable
awards while in the process.
While checking for the key in the Request object is highly inelegant,
it is consistent with how we determine elsewhere if rendered comments
are being shown as a context.
Pending a more thorough comments rework, make comment listings denser:
- Reduce margin between comments from 28px to 3.5px.
- Reduce padding around comment text from 10px to 5px.
- Shrink comment-actions text size by 20% to maintain visual balance.
- Add some padding to frontpage list entries, because the size of
voting arrows was holding the size up. Should be about the same
as before action icons were shrinked.
Also wound up semi-unrelated fixing the comment nesting border-left
colors on TheMotte mobile, coffee, win98, dramblr, and midnight.
All changes tested most thoroughly on those themes.
The last places coins were visible to non-admins were in user popovers
and, potentially, via direct access of /@<username>/coins. These have
been removed.
Additionally, there are a number of routes which, despite being removed
from the UI, were still operative and usable. These are disabled
pending possible(?) future uses of coins.
Given that coins are not visible in many contexts, the conspicuous
appearance of treasure chests (random coin rewards on 1% of comments)
seems out of place. This removes the logic which rewards treasure,
the visible display of treasure, and drops the column containing
treasure information which has already been awarded to at least one
comment on prod.
Using the new assetcache module, we replace (almost) all instances
of `?v=` cachebusting query parameters in Python and in Jinja templates.
The primary exceptions were: user site backgrounds, and some
infrequently changed graphics referenced literally from .js files.