Conversation
Now display the lastEditedName for EOR reasons on `run-detail` page. `formatEorReason` also renders a tooltip above the `lastEditedName` to make clear who it refers to.
Adjust expectations to include displayed author 'Anonymous' on a new line.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2090 +/- ##
==========================================
- Coverage 45.58% 45.56% -0.02%
==========================================
Files 1040 1041 +1
Lines 17310 17317 +7
Branches 3134 3136 +2
==========================================
Hits 7891 7891
- Misses 9419 9426 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add a dedicated eorReasonComponent that renders the EOR reason VDOM.
Stop keeping a separate `eorReasonLastEditedNames` list on RunDetailsModel and instead include `lastEditedName` on each eorReason within RunPatch. RunPatch `toPojo` then strips that field when building the outbound patch. This removes duplicate state and ensures only allowed fields are sent to the server.
…son-on-run-details
Rename eorReasonComponent to formatRunEorReason. Return null instead of empty string for missing lastEditedName so the vdom doesn't render an empty node.
Adds a new test in that verifies the EOR reason popover shows the expected tooltip text.
…son-on-run-details
There was a problem hiding this comment.
PR looks good but I think we should take into consideration the wide-screen scenario as most of the screens at P2 are like that. Right now it is inconvenient to check left side screen with right side to see the author.
What do you think about bringing the author next to the EOR perhaps something like <eor_reason> added by ? Moreover, I think the tooltip does not need to be used in this proposal.
| import { formatEorReason } from './formatEorReason.mjs'; | ||
|
|
||
| /** | ||
| * Display the given EoR reason as a vdom component with lastEditedName tooltip |
There was a problem hiding this comment.
I am guessing you meant vnode instead of vdom?
In my opinion I think it is rare people are opening BKP at the wide-screens' full-width given the amount of whitespace and eye movement this causes with all the other UI elements not just EOR reasons section. I also think if you add more onto this left side, we will have to redesign how EOR reasons appear as it would be 4 data points separated by dashes or even another way but still close together and at that point I think this is not good UX either. And redesigning is a larger ticket given this is so used etc. Given you are already reading left to right, and it is never a huge list of reasons, then I thought a keeping to the right was okay. Re the tooltip I thought it was useful just to reiterate to the user what this information was because I am unsure if they even know that EOR reasons are recorded with who created/editied them. |
|
Very well then we continue like this. My comment on spacing was due to Filippo's recent comments on spacing in the filter boxes between label and input widgets as to there being too much white between them. |
I completely understand, for reference I add below what, very crudely, adding the name to the left side would look like. 
|
…son-on-run-details
…details' of github.com:AliceO2Group/Bookkeeping into improv/O2B-1536/Add-lastEditiedBy-to-EOR-reason-on-run-details
…son-on-run-details
graduta
deleted the
improv/O2B-1536/Add-lastEditiedBy-to-EOR-reason-on-run-details
branch
2 participants
I have a JIRA ticket
Notable changes for users:
Notable changes for developers:
Changes made to the database: