use a dl for achievement item list

[Alex-Jordan]

This changes the list of earned achievement items to use a dl instead of a sequence of h3 tags.

Additionally, I believe this fixes some maketext issues where it was previously applied directly to a variable representing the name of an achievement item.

The badge section is also changed to indent at the same level as the item list.

And then some things from the achievements css are removed because they are no longer used.

[somiaj]

I think $item->name isn't translated here, shouldn't this be to ensure translation on the name?

				% $button_text = maketext('[_1] ([_2] remaining)', maketext($item->name), $itemCounts->{ $item->id });

I thought name => x(string) only flags the string to be translated, but it won't be translated until it is run through maketext()? I think as a result you have also removed translation in a few places below as well (or am I not understanding how the x(....) creates translations).

This is at least in contrast to lines 11 and 14 where you do run the name and description through maketext.

[somiaj]

I do know there was some issue with nested maketext translations, but in this case I don't think it is an issue since we aren't translating a raw string, but the value of a variable that has been previously marked for translation?

[Alex-Jordan]

In the current .pot file, I only found things like "Scroll of Resurrection" referenced from the item's pm file, and not from this template. I could be wrong. But if that's what I saw, to me that said these were not being translated here. Out of the house now, can't test.

…

On Sun, Aug 18, 2024, 2:44 PM Jaimos Skriletz ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In templates/ContentGenerator/Achievements/achievement_items.html.ep <https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-16e2924345da4f71&q=1&e=b8df15ef-0e5c-4d71-9eda-a63f2177db5f&u=https%3A%2F%2Fgithub.com%2Fopenwebwork%2Fwebwork2%2Fpull%2F2529%23discussion_r1721067915> : > - <h3><%= maketext($item->name) - . ' (' . maketext('[_1] remaining', $itemCounts->{ $item->id }) . ')' %></h3> + % $button_text = maketext('[_1] ([_2] remaining)', $item->name, $itemCounts->{ $item->id }); I do know there was some issue with nested maketext translations, but in this case I don't think it is an issue since we aren't translating a raw string, but the value of a variable that has been previously marked for translation? — Reply to this email directly, view it on GitHub <https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-16e2924345da4f71&q=1&e=b8df15ef-0e5c-4d71-9eda-a63f2177db5f&u=https%3A%2F%2Fgithub.com%2Fopenwebwork%2Fwebwork2%2Fpull%2F2529%23discussion_r1721067915>, or unsubscribe <https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-c49f2888fdf425c8&q=1&e=b8df15ef-0e5c-4d71-9eda-a63f2177db5f&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABEDOAHGU6NM3PXNJO2B2BDZSEISFAVCNFSM6AAAAABMWV65MOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENBUGQYTCNJZGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[drgrice1]

Yes, @somiaj is correct. The x method does not do any translation. It is just a marker so that the xgettext.pl script will add the string to the .pot file. Note that the .pot (and .po) files will only show the file and line number for the file where the x call is made, NOT where the translation is done. This is because the xgettext.pl only sees where the x call is made on the string, and doesn't see where the actual maketext call is done on the variable value that the result of the x call is stored in.

Also, the maketext call must be called only on what is put into the x method. So @somiaj's suggested change for line 19 is correct, and nesting the maketext will work in this case. Lines 21 and 23 will also need to be changed accordingly.

[drgrice1]

By the way, the dl restructuring generally looks good. However, I think that the font size of the reward item name is a bit large. You are using fs-3 for that. I would suggest using fs-4. The fs-3 font size is barely less than the font size used for the "Rewards" text in the h2 header for this section. Also, it is considerably larger than the fs-5 font size used for the badges below.

[Alex-Jordan]

Thanks @somiaj, @drgrice1. I made those changes.

[somiaj]

Overall looks good. I initially thought the number of items (4 remaining) looked odd inside the button, but I can't think of a better place to put that info.