Opened 6 years ago

Implement group toggling behavior for checkboxes on Account: Cleanup page

Reported by: Owned by: Ryan J Ollos normal AccountManagerPlugin normal jQuery

Description

As was done in #10745 for the Manage User Accounts page, implement group toggling behavior for the "parent" checkboxes on the Account: Cleanup page.

comment:1 follow-up:  6 Changed 6 years ago by Ryan J Ollos

Patch in t10755. I'd like to redesign the layout of the page, but I'll leave that for another ticket.

Let me know if you have any trouble working with the changesets posted to GitHub. I made a few comments about that comment:3:ticket:11071.

comment:2 follow-up:  3 Changed 6 years ago by Steffen Hoffmann

I understand the target of this request. Here are some thoughts:

The analogy on the accounts/user list does not hold, when it comes to the 'unselect all' button. IMHO we have no good alternative to it without JS enabled. You should only disable/hide that button, if you can be reasonably sure, that you'll not regress on the functionality. Trac does not assume JS enabled for now, nor do I for now.

I'll consider your thoughts on a page re-design, sure. Because that page sprang solely from my wish of playing nice with the more web-UI affine kind of admins when it came to managing the 'session_attribute' table, it has been put out as it was. It was a development stage for the list pager. It was short-cut to verify certain attributes without the need to go and read db content directly.

Frankly, with a clear account/user delete procedure in place I feel much less need to invest effort into that page right now. I hope you'll glean more about that at least from coming changes for #11090. This is a rather big, concentrated effort towards getting account notifications right. I feel, that it's time to clear that up, because it has been working more or less by chance until now. Now I'm tightening the grip on that part of the code base, eventually jumping to the AnnouncerPlugin domain every now an then.

comment:3 in reply to:  2 ; follow-up:  7 Changed 6 years ago by Ryan J Ollos

The analogy on the accounts/user list does not hold, when it comes to the 'unselect all' button. IMHO we have no good alternative to it without JS enabled. You should only disable/hide that button, if you can be reasonably sure, that you'll not regress on the functionality. Trac does not assume JS enabled for now, nor do I for now.

I'll change the code to retain the button so that we don't lose the functionality when JS is disabled. However, Trac no longer aims to provide full functionality without JS. See t:#11014 for details.

The page redesign I had in mind would be post-0.5 release.

comment:4 follow-ups:  5  10 Changed 6 years ago by Ryan J Ollos

Suggested change implemented in t10755.2. Thanks for the hint, hiding the Undo selection submit button seems like a better solution.

comment:5 in reply to:  4 Changed 6 years ago by Steffen Hoffmann

Suggested change implemented in t10755.2. Thanks for the hint, hiding the Undo selection submit button seems like a better solution.

Yes, that's pretty much everything, that is required for now to clean the web-UI a bit for now. Thanks.

comment:6 in reply to:  1 ; follow-up:  9 Changed 6 years ago by Steffen Hoffmann

Patch in t10755. I'd like to redesign the layout of the page, but I'll leave that for another ticket.

Let me know if you have any trouble working with the changesets posted to GitHub. I made a few comments about that comment:3:ticket:11071.

Actually a very minor one: Is there an easy way to get a plain text diff output of changes? I miss it for copy-pasting bigger code fragments, at least until I've learned about and setup a hg-git bridge to pull from GitHub into Mercurial directly.

comment:7 in reply to:  3 ; follow-up:  8 Changed 6 years ago by Steffen Hoffmann

The analogy on the accounts/user list does not hold, when it comes to the 'unselect all' button. IMHO we have no good alternative to it without JS enabled. You should only disable/hide that button, if you can be reasonably sure, that you'll not regress on the functionality. Trac does not assume JS enabled for now, nor do I for now.

I'll change the code to retain the button so that we don't lose the functionality when JS is disabled. However, Trac no longer aims to provide full functionality without JS. See t:#11014 for details.

A very interesting lecture on current Trac development attitude. Trying to follow good practice as well, I'd really value, if core developers would disclose a bit more of their Trac foo, so we could raise/level Trac plugin standards too. To me it seems a bit short-sighted, risking to spend much more time on arguments later on instead of bringing such policy questions out into the open and make the point clear just once. That would boost value of TracWiki below TracDev/ too.

The page redesign I had in mind would be post-0.5 release.

Fine.

comment:8 in reply to:  7 Changed 6 years ago by Ryan J Ollos

A very interesting lecture on current Trac development attitude. Trying to follow good practice as well, I'd really value, if core developers would disclose a bit more of their Trac foo, so we could raise/level Trac plugin standards too. To me it seems a bit short-sighted, risking to spend much more time on arguments later on instead of bringing such policy questions out into the open and make the point clear just once. That would boost value of TracWiki below TracDev/ too.

I was a bit surprised by the discussion as well. I haven't formed a solid position on the issue yet - on the one hand I feel that graceful degradation is a very good practice, but on the other hand there are probably very few users that run a browser without JavaScript, and it might not be the best use of our time and efforts to make all of the features work without JavaScript.

However, BatchModify is a pretty major feature, and I tend to think it should work without JavaScript enabled. As far as I know it is the only major feature that requires JavaScript. The BatchModifyPlugin was integrated with a pretty minimal amount of modification into the Trac core. I wonder if there was any profiling done to determine if there would really be that big of a performance hit using Genshi. I tend to think that wouldn't be the case because, unlike the original BatchModifyPlugin which would have had to implement an ITemplateStreamFilter to modify query.html, the Trac devs could have just added the checkboxes directly to query.html. You might imagine, I was thoroughly discouraged from pursuing any of these questions on my own by the replies given in that ticket. It might be an interesting exercise to see if the feature could be implemented efficiently without JavaScript, but after the response given in that ticket I couldn't expect any patches I generated to be given any consideration.

comment:9 in reply to:  6 Changed 6 years ago by Ryan J Ollos

Actually a very minor one: Is there an easy way to get a plain text diff output of changes? I miss it for copy-pasting bigger code fragments, at least until I've learned about and setup a hg-git bridge to pull from GitHub into Mercurial directly.

How to do so was not obvious to me, and I didn't find an answer in the GitHub help docs. After some googling, I found a hint that said appending .diff to a url will give a plain text diff. It works, f4ae0c13.diff.

comment:10 in reply to:  4 Changed 5 years ago by Ryan J Ollos

Suggested change implemented in t10755.2. Thanks for the hint, hiding the Undo selection submit button seems like a better solution.

It looks like I accidentally pushed an unrelated change at the end there, in 9391be056, which you'll want to ignore.

comment:11 Changed 2 years ago by Ryan J Ollos

Owner: Steffen Hoffmann deleted

Modify Ticket

Change Properties