Skip to content

Added the ability to disable group similar time entries#1054

Merged
Onatcer merged 6 commits intosolidtime-io:mainfrom
utlark:feature/disable_group_similar_time_entries
Apr 17, 2026
Merged

Added the ability to disable group similar time entries#1054
Onatcer merged 6 commits intosolidtime-io:mainfrom
utlark:feature/disable_group_similar_time_entries

Conversation

@utlark
Copy link
Copy Markdown
Contributor

@utlark utlark commented Apr 5, 2026

What does this PR do?

Adds the ability to disable grouping similar time entries. #445

Checklist (DO NOT REMOVE)

@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented Apr 7, 2026

thanks for the contribution. a few notes:

  • The grouping currently only exists in the time view, i think it makes more sense to have the setting there directly. a cog icon ghost button on the right side of resources/js/packages/ui/src/TimeEntry/TimeEntryMassActionRow.vue with a popover and the setting inside should work fine i think. (the goToProfilePage which is duplicated from profile.spec.ts and the profile.spec.ts test can be dropped then)
  • there are a few page.waitForTimeout(500) in the tests. are those really necessary, usually you should just wait for the element in playwright because its a lot faster.
  • the groupSimilarTimeEntriesSettings useStorage key should not have a "theme-" prefix, should be singular (groupSimilarTimeEntriesSetting) and should be separate from the theme.ts . i don't think any of the existing utils are a good fit, it can just be a timeEntryGrouping.ts.
  • the Ternary in TimeEntryGroupedTable is pretty hard to read imo this should be an early return.

@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented Apr 7, 2026

i just realized the TimeEntryMassActionRow is used in the Reporting as well. We can just add a "settings" slot to the component and only pass it in Time.vue. The useStorage can stay in the main application then and can be passed down like the PR already does.

@utlark
Copy link
Copy Markdown
Contributor Author

utlark commented Apr 7, 2026

Alright, I’ll try to move the setting closer to the Time view. I’ll also adjust how storage is handled (key, naming, extracting it into a separate file) and replace the ternary with a more readable early return.

Regarding the tests — without waitForTimeout they either didn’t work at all or behaved inconsistently.

Observations by case:

  • On the first call of goToTimeOverview(page), waiting is generally not required.
  • When calling createEmptyTimeEntry(page) twice, the timer for the second entry does not stop and keeps running. The added wait ensures that the entry is reliably created and stabilized.
  • After the second goToTimeOverview(page), the page opens but the data is not fully loaded yet, while the test already starts searching for page.locator('[data-testid="time_entry_row"]'), which results in the element not being found and the test failing.

I agree that waiting by timeout is more of a workaround, but I was not able to find a stable alternative based on waiting for specific states. I’m not very familiar with writing Playwright tests, so I might be missing a better approach here.

@utlark
Copy link
Copy Markdown
Contributor Author

utlark commented Apr 14, 2026

I got some time to work on this and took a closer look at TimeEntryMassActionRow, similar solutions, and the overall UI. I tried to follow the proposed approach, but during implementation some UX concerns came up.


Regarding TimeEntryMassActionRow

On the time view, this component appears to be redundant.

It enables bulk editing across all selected entries. In practice, this implies applying a single template change to a large time range (e.g., a month), which seems like an unlikely use case.

There is also a UX issue:
if a user selects entries deep in the list, the controls appear only at the top (TimeEntryMassActionRow), requiring scrolling back up to interact with them.

A more consistent approach would be to perform edits at the group level (e.g., per day). There is already TimeEntryRowHeading, which aligns better with this model:

  • actions apply to a limited, logically grouped set of entries
  • controls are placed close to the relevant data
  • no additional navigation (scrolling) is required

Possible options:

  • fully move bulk editing functionality on the Time page into TimeEntryRowHeading
  • or keep TimeEntryMassActionRow, but also introduce similar functionality in TimeEntryRowHeading

In comparable applications, bulk operations are typically scoped to a group (day), rather than applied globally across all selected entries.


Summary

Before moving the grouping setting into TimeEntryMassActionRow, it should be clarified whether this component will remain on the Time page.

Changes related to reconsidering the role of TimeEntryMassActionRow are outside the scope of this PR. If needed, I can take this on as a separate task and implement it.

@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented Apr 15, 2026

I agree that the MassEditRow in the Time view has problems, specifically that it can be out of the viewport when selecting time entries and that "Select All" is not a useful scope for an infinite scroll page. I'm honestly not a big fan of the approach that Clockify and Toggl are doing either tho, because everything is day scoped you'd need multiple bulk actions to edit time entries across days. maybe I'm missing something but i think dropping the select all on the time page and having a sticky MassActionBar at top when entries are selected is a better way to handle it. Selecting all time entries of a day is already possible via the TimeEntryRowHeader. Having the Edit and Delete Actions directly in the header adds ambiguity imo because as soon as time entries from multiple days are selected, it's not really clear anymore what the bulk actions are performed on. The component is also used int he Detailed reporting where it works from a UX perspective imo.

But really this is a separate discussion, I created one here to discuss this. For this PR I'm fine with keeping it in the Profile Settings.

@utlark
Copy link
Copy Markdown
Contributor Author

utlark commented Apr 15, 2026

Most feedback has been addressed in the latest commits:

  • UI logic updated: ternary in TimeEntryGroupedTable replaced with early return
  • Storage handling refactored: renamed key and moved logic into a separate module
  • E2E tests updated: replaced waitForTimeout with element-based waiting
  • Lint/format applied

For the setting placement, we agreed to keep it in Profile Settings for now.

@Onatcer Onatcer merged commit 77a5e97 into solidtime-io:main Apr 17, 2026
1 check passed
@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented Apr 17, 2026

LGTM, thank you for the contribution!

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.

2 participants