Skip to content

Fix layout in Redmine 4.1, add timestamps to merge requests #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexandermalfait
Copy link

Relating to #26

@alexandermalfait
Copy link
Author

Example layout:

image

@tf
Copy link
Owner

tf commented Sep 25, 2020

Sorry for not getting back earlier. This might be a good compromise. I find the new design a bit dense and harder to read. Have you considered keeping the items multi line or turning the list into a table to align columns? I also asked for design feedback internally since this is a plugin we use quite a lot.

CI is failing since the migration is not compatible with all supported Redmine versions. You can check the other migrations for the correct syntax.

How does the timestamp look like for old merge requests that were added before the timestamp columns were added?

Also if we take this route, we will no longer depend on the custom hook that currently needs to be added via a patch. The corresponding files could then be removed and the readme would need to be updated accordingly.

@aviav
Copy link

aviav commented Sep 25, 2020

Kudos for coming up with a fix! I also use this quite a lot, but currently, we are on an older version of Redmine, so that's why the issue hasn't yet occurred for us.

For me, this change would introduce the problem that it is easy to lose track which line I am in while scanning the long line, and then mis-identify which PR/MR ID is open, which is merged, or which is closed. As your example shows, MR titles could be quite similar and are sometimes even identical which makes it even more likely to slip between lines. In this use case, having ID and MR status close together makes it much easier to quickly and reliably tell what is the status of which. In practice, I use these IDs to distinguish between MRs when communicating with others, and often, only a minority of MRs/PRs on a ticket is actually open, so having these bits of information close together is great for my use case.

A potential fix for this problem might be to alternate background color slightly e.g. between odd and even lines -- but still, the lines would be pretty long, full of information, and for me, the information that, in the best case, would be close together is now far apart.

In principle, I like the layout that we had up until now, because it didn't increase the need to scroll by too much. The change introduced in this PR also achieves this well. By the multi-column approach with comments etc. in one column and MRs/PRs in another column, however, lines in both columns became shorter and easier to scan.

Still, for the tab layout, which I think is a very nice addition and which I look forward to using, I don't even know whether it's possible to maintain all those advantages at the same time. Maybe just having MR/PR information on all the tabs would be fine for now, though we should fix the styles in this case. I'd expect that it could turn out that at least I only need PR/MR info on the tab where the discussion takes place, if I want to avoid having to switch tabs -- but this is only a guess, since I haven't used these tabs yet. How do you use this information, usually?

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.

3 participants