Skip to content

Conversation

@seanmarcia
Copy link
Member

WIP

@dcollie2
Copy link
Collaborator

dcollie2 commented Oct 8, 2025

We'll want to get the tag and file management interfaces back in shape.
image

seanmarcia and others added 17 commits November 10, 2025 14:42
- Replace custom pagination with Pagy bootstrap helper and ensure search filter params persist across pages
- Update search ordering to use published_at instead of created_at for topics
- Fix password reset page link text expected by tests (Login) and restore test description text
- Adjust Searcheable concern year/month scopes to handle blank params safely
- Ensure state filter respects enums in Searcheable#by_state
- Force immediate form submission for month and state selects to avoid stale content in tests
We do not have a surver/privacy policy/terms of use and only registered
users can log in, so I removed those links and the card that made it
sound like anyone could sign up.
With the old design, we were filtering out archived topics by default.
@Oli0li Oli0li force-pushed the tailwinds_conversions branch from 850fe0c to a6811e4 Compare November 10, 2025 18:03
@Oli0li
Copy link
Collaborator

Oli0li commented Nov 10, 2025

I've resolved the conflicts/removed some unnecessary links, still need to fix:

  • pagination
  • tags
  • document upload interface
  • "Topic statistics" not aligned on Topic form
  • Background colour for rows on Region charts (hard to read until we hover on a row)
  • I think we can remove the capital letter on a purple background on the Topic list, it wasn't clear to me that this was the first letter of the Topic name, which led me to wonder if this was some kind of code

This was causing the content of the table to be displayed by default
on a green background, making it hard to read.
It wasn't clear to me what it meant when I saw it. Since it is not some
kind of code to help idenfity a certain category of topic, I think it is
best to remove it to avoid confusion and leave more space for the title.
@Oli0li Oli0li force-pushed the tailwinds_conversions branch from 82f5572 to 5afd19b Compare November 18, 2025 19:07
@Oli0li Oli0li force-pushed the tailwinds_conversions branch from 65481bf to ef035af Compare November 29, 2025 19:45
</footer>

<style>
@keyframes fade-in-up {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to load this as an asset?


content_tag :nav, 'aria-label': "Page navigation" do
content_tag :ul, class: "flex -space-x-px text-sm", style: "list-style: none; margin: 0; padding: 0;" do
safe_join([
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this to connect page with tailwind?

<%= render "shared/errors", errors: language.errors.full_messages, resource_name: language.class.name %>

<div style="margin-bottom: 1.5rem;">
<%= f.label :name, style: "display: block; font-size: 0.875rem; font-weight: 600; color: #374151; margin-bottom: 0.5rem;" do %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use inline styles and not classes?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it would be a lot more readable and it would help ensure we have consistent stylings across the app

});

// Smooth scroll for anchor links
document.querySelectorAll('a[href^="#"]').forEach(anchor => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should live in JS module that we can load here

Oli0li added 10 commits December 6, 2025 15:33
Removing the ability to add new Tags from the Tag index was a conscious
decision we took because we didn't want Tags that were not used on any
Topics, and at the time it would not have been possible to determine
for which language a Tag was meant. Now that we display all Tags
regardless of a Topic's language, the language consideration would no
longer be an issue, but the stakeholders haven't told us they needed
this feature back.

See the PR where we removed the ability to add Tags from the Tag index:
8f77030
This means we use overmind instead of foreman.
Users can already see this info on the form or when clicking on
"View Topic Details" at the bottom of the form.
Red is generally displayed for destructive actions, but here we were
displaying "Edit" in red and "Archive" in orange. Since we are using
orange on the Edit forms elsewhere, I've used orange for the Edit button,
gray for Archive, and red for Delete. But orange is also often used to
indicate a mildly dangerous action, so I wonder if it should be changed
to something less alarming, like blue, purple or green. But editing a
Topic can indeed be slightly dangerous if the user makes a mistake, so
it could also be argued that orange is appropriate.
I also removed the "Danger Zone" as I thought it was a bit confusing
that it showed warnings for a Delete feature that does not exist.
If the stakeholders need this feature, they will let us know and we
can add it at that point.
Admins can already see that information on the form itself or
by clicking on the "View Provider Details" button below.
Copilot AI review requested due to automatic review settings December 6, 2025 17:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Oli0li
Copy link
Collaborator

Oli0li commented Dec 6, 2025

I've asked Claude to help convert a lot of the repetitive inline styles into Tailwind classes, but there are still many files to cover.
On top of some of @dmitrytrager's comments I still need to address, I've also noted this should be changed:

  • Add spacing on Providers#show
  • For Providers forms, maybe use the same style we have to select several regions as we have on the User form

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants