liqd / adhocracy-plus

adhocracy+ is a SaaS platform to make digital democracy easy for everyone
https://adhocracy.plus/
GNU Affero General Public License v3.0
73 stars 20 forks source link

[8312] comments: refactor and adjust comment styling to new a4 comments #2592

Closed goapunk closed 1 month ago

goapunk commented 2 months ago

depends on https://github.com/liqd/adhocracy4/pull/1601

there's no design yet with the changes but hopefully it's ok like that

test: on any module which has comments

WIP because it needs the a4 pr merged first and then the a4 hash updated here

also fixes #2553

Tasks

m4ra commented 2 months ago

hmm, the button post doesn't have a background color. The alert that comment was posted should be on the top, as it is now in the section of the comments, which starts rather low and alert can be missed. I think is best for @hom3mad3 to do the review here and propose some UI solutions. Or have a design from Marcelo.

goapunk commented 1 month ago

@m4ra the post button has a background color for me, could you give more info on where it's broken (which module, etc) ? I agree that t he alert isn't ideal but it's also not great on dev either, so maybe we ignore it for now or make an issue as we do need this pr for the release

m4ra commented 1 month ago

@goapunk I don't see it broken now with the mapidea and document modules, I should have noted down which module. There are two little things we can do html wise that can make the big space less and render the success alert more visible.

  1. In your PR here, change the div class from my-4 to my-1 in line 159 of the file apps/contrib/templates/a4_candy_contrib/item_detail.html
  2. Make the header of Discussion h3 instead of h2, same as the header Join the Discussion in the a4 PR . This will also solve the issue of the different fonts between these headers I see now happening.

Screenshot from 2024-05-21 15-02-14

goapunk commented 1 month ago

@m4ra for 1. changing my-4 to my-1 will also shrink the margin at the bottom, not sure if we want this? for 2. I'm unsure if changing to h3 is a good idea accessibility wise, I think technically both of them should actually be h2 instead of h3 to create a logical structure?

At this point we should probably ask @mcastro-lqd how to best proceed then. @mcastro-lqd The problem for the new comments is that the alert is quite further down when posting a new comment via the "Join the discussion" form (see screenshot below). I'm also now unsure if the headings are correct (see the screenshot as well). Any suggestion what we should do here?

Screenshot 2024-05-21 at 14-39-07 adhocracy _liqd-orga

goapunk commented 1 month ago

@m4ra @mcastro-lqd The same technically also applies to meinBerlin, we need to keep that in mind when making any changes here

mcastro-lqd commented 1 month ago

Would "using a CSS Framework utilizing utility classes to adjust the heading size. For example, with Bootstrap, you can use classes like h3, h4, etc., to adjust the heading size while maintaining semantic <h2> tags." be the case? Cause otherwise it's too big.

goapunk commented 1 month ago

@m4ra @mcastro-lqd I changed both to be h3 now, so it's a bit smaller

goapunk commented 1 month ago

and decreased the margin-top for the post button as it was much bigger than on dev

goapunk commented 1 month ago

@m4ra also fixed the margin-top now

goapunk commented 1 month ago

@m4ra opened https://github.com/liqd/adhocracy4/pull/1601 so this can be reviewed again