Highlight selected file version (in Nextcloud Office and other similar context)

[Jerome-Herbinet]

Adding same css behavior as hover.
Fixes UX issue when user often doesn't know what history file version has been selected previously.

The "active" CSS class already exists and is already added/removed on the "li" tag. The only missing stuff is the CSS property that i have added in this pull request.

[Jerome-Herbinet]

For reviewers : Impact of new CSS property on front rendering.

[nimishavijay]

Very nice work! I only have some small suggestions:

• in the screenshot it seems like the color is full opacity black #000000. We rarely ever use this, and I would suggest var(--color-main-text) which is #222222
• We should also change the color of the text in the subline (8KB) and the color of the restore button to var(--color-main-text) as they all belong to the same group
• If possible, we could change the background color to var(--color-primary-light) and round the corners like it is in Talk, but it already looks pretty cool :)
  

[Jerome-Herbinet]

Very nice work! I only have some small suggestions:

* in the screenshot it seems like the color is full opacity black `#000000`. We rarely ever use this, and I would suggest `var(--color-main-text)` which is `#222222`

* We should also change the color of the text in the subline (8KB) and the color of the restore button to `var(--color-main-text)` as they all belong to the same group

* If possible, we could change the background color to `var(--color-primary-light)` and round the corners like it is in Talk, but it already looks pretty cool :)
  ![image](https://user-images.githubusercontent.com/52440189/169516489-f70397a9-929f-4956-af2c-82d0d1adfff0.png)

@nimishavijay , thank your for your reply !
I've made some changes. Can you check rendering and give me your opinion ?
Thanks by advance ;-)

When "active" class is enabled :

• Background color (with var(--color-primary-light)) + border radius (16px), like in Nextcloud Talk
• Subline color: var(--color-main-text)
  Consequence of this change :
• Slight shift of the document preview thumbnail and the restore button so that they are not stuck to the edge of the frame when it is light blue.
• Removal of the light grey border that was applied to the "li" tag as it followed the newly added rounding.

Bonus :

• Improved the visibility of the thumbnail with the addition of a grey var(--color-border-dark) border as it was not very visible on the white background of the interface outside the general context of this pull request.

[nimishavijay]

Please post screenshots so it is easy for everyone to review design changes :)

[Jerome-Herbinet]

Please post screenshots so it is easy for everyone to review design changes :)

[nimishavijay]

Super nice! Looks great now! Only some issues with alignment which are more visible with the colored background:

• the restore button should be vertically centered and moved to the left such that it has equal space in its top, bottom and right
• Move both the lines of text (including the download icon) 2px to the right because it looks too close to the filetype icon
• I'm also thinking the subline text (8KB) could be moved 1 or 2px upwards to have more space below, since there is no border to separate the list items

[Jerome-Herbinet]

I'll take care of that next week.
Have a nice week-end !

[Jerome-Herbinet]

@nimishavijay , here are the last changes.

[nimishavijay]

Looks perfect! No other comments from my side! Really really nice work :) 🚀

[Jerome-Herbinet]

@jancborchardt @nimishavijay ,
Another issue I'm going to create in parallel (unless you already know an existing issue) is that the "class" named active is not applied to any of the <li> tags after the first page load (after clicking on "see history"). In other words, it is the first <li> tag in the list that should have the active class as soon as the page is loaded, without having to click on it.

[artonge]

The code could probably be refactored to be more simple, but if it works, then ok :)

[ChristophWurst]

Something went wrong with the rebase :/

[zak39]

Something went wrong with the rebase :/

Yeah, I made a mistake in helping Jérôme with his PR...

We have rebased the branch main from his branch (patch-1) yesterday.
But, this morning, I understood we have to write Signed-off-by: Random J Developer <[email protected]> in his commits with git rebase command :/

So, you think we have to resolve conflicts or create a new PR ?

[artonge]

So, you think we have to resolve conflicts or create a new PR ?

Preferably clean this PR, but if it is too much work, you can create a new PR.

Not sure how you ended up in this state, but I would do something like to recover:

git checkout patch-1
git checkout -b patch-1.bak
git branch -D patch-1 # Destructive step on local repo
git checkout master
git checkout -b patch-1
git cherry-pick 4d61008caeff0db760c2f4aa52e698251863b2b1
git cherry-pick 85279020f2c5dd6cfed48d410bde03203b8984d5
git cherry-pick 699efb45fa6e16317f0b14cd915ffd8c2eb9c2b0
git push --force # Destructive step on remote repo

But be careful to not lose your work !

[Jerome-Herbinet]

I close my messy PR and I will create a new one later. Have a nice day.

[Jerome-Herbinet]

New PR #33434 @nimishavijay
I've finally added my signature in my commit :-)
I cross the fingers (it finally should work).

[Jerome-Herbinet]

@jancborchardt @nimishavijay , Another issue I'm going to create in parallel (unless you already know an existing issue) is that the "class" named active is not applied to any of the <li> tags after the first page load (after clicking on "see history"). In other words, it is the first <li> tag in the list that should have the active class as soon as the page is loaded, without having to click on it.
Here is the issue : nextcloud/richdocuments#2396