1599297 - Keep details header fixed while scrolling

Status: NEW

(Toolkit :: Add-ons Manager, enhancement, P3)

Description

[Maverick]

Attachment: fixedheader.jpg

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

accessed about:addons and accessed an addon page (e.g. ublock)

Actual results:

after reading some details and configure if i allow updates and run in private i had to scroll up just to change to release notes (or permissions)

Expected results:

addon's page header header should be fixed / details should be inside a iframe

Comment 1

[Mark Striemer [:mstriemer]]

Thanks for the suggestion. I've added it to the agenda of our UX/product discussion next week.

Comment 2

[Maverick]

Hi. No need to thank me :)

by the way, in that same page, the options "allow automatic updates" and "run in private windows" are also a bit out of hand.

For a user that wants to configure those options it has to go chase them.

Would be nice to have them on the top of the details tab or in a dedicated tab (or maybe move those to the permissions tab - i know it's an informative tab, but i would look there...)

(I did not create a bug for this because i think i've read somewhere the intention is to "force" the user to read the addon details and only then configure it's permissions, but i maybe wrong.)

Comment 3

[Mark Striemer [:mstriemer]]

This was well received with the product team. We had previously discussed setting a max height on the description and adding a "Show More" button to the description but didn't end up exploring it much in the past. That should help make the options at the bottom of the page more visible.

I think we'll want to do both the fixed header and the max height on the description and then run this past the UX team and see if they have any comments or suggestions.

Comment 4

[Maverick]

Great to ear that! :)

Comment 5

[Mark Striemer [:mstriemer]]

Emanuela added a mockup for the description which we already had on file (bug 1565271). Making this about just the header again and linking to that one.

Comment 6

[Maverick]

Does this have any ETA or it's more kind of "when you finish other more important things"? :)

Comment 7

[Marinella]

Hi Maverick could you assign this bug to me?

Comment 8

[Maverick]

@Marinella Hi.

I'm just the original reporter. I don't think i have those "powers"...

Comment 9

[Maverick]

I was thinking about it, Mark will most likely be able to help you on this... I'll leave a needinfo to him.

Comment 10

[Marinella]

(In reply to Maverick from comment #9)

I was thinking about it, Mark will most likely be able to help you on this... I'll leave a needinfo to him.

Thank you!

Comment 11

[Mark Striemer [:mstriemer]]

You're welcome to work on this, you don't need to have the bug assigned to work on it. Once a patch is attached to the bug then it will automatically get assigned to the author.

Comment 12

[Marinella]

(In reply to Mark Striemer [:mstriemer] from comment #11)

You're welcome to work on this, you don't need to have the bug assigned to work on it. Once a patch is attached to the bug then it will automatically get assigned to the author.

Perfect!

Comment 13

[Marinella]

Attachment: DOM change possible solution

I’ve tried to solve the problem, but I’m not experienced enough with JS and in a complex structure like this I can’t place or address it properly. The code would work in a simple structure. I place it anyway in case it could be useful as a starting point for someone.

Comment 14

[Jayati Shrivastava]

hello, May I give this a try?

Comment 15

[Maverick]

any progress on this?

Comment 16

[Priyank Singh]

Hi Mark,
Could you list the files that needs to be changed to implement this feature.

Comment 17

[onuohaoluebube05]

Hello, I am an outreachy applicant, would love to work on this bug.

Thanks

Comment 18

[Maverick]

(In reply to onuohaoluebube05 from comment #17)

Hello, I am an outreachy applicant, would love to work on this bug.

Thanks

I think you can just start working on it and submit a patch for review :)

Comment 19

[Blessing Peters]

Hello, I am an outreachy applicant and would love to work on this bug.

Comment 20

[WhiteMind]

I am prepared to take over this issue and have already come up with a solution.

There are two implementation options based on the page structure:

Implement using CSS variables, which requires recording the height of addon-page-header in CSS variables.
Split addon-details into addon-details-header and addon-details-content, then elevate addon-details-header to the same level as addon-page-header and use the same sticky-container.
Here are the issues with these two options and my choice:
Option 1 cannot be implemented purely with CSS (such as --addon-page-header-height: --partAPadding + --partAHeight + --partAHeight --partBPadding ... etc.), as this would place many restrictions on future development (maintainability), such as difficulty in adding dynamically sized elements and difficulty in implementing responsive line breaks. Therefore, Option 1 must use JS helpers such as ResizeObserver.
For Option 2, I have made some attempts, but elevating the layer of addon-details-header is too much. Originally, addon-details was managed under the main(view) -> addon-card hierarchy, and the selectedAddon status was also managed in main (https://hg.mozilla.org/mozilla-central/file/a445f1762c895000bcdabd9d95697522359d41ed/toolkit/mozapps/extensions/content/aboutaddons.js#l4118), and main is at the same level as addon-page-header. Elevating addon-details-header to the same level as addon-page-header would increase the complexity of state management, which would affect maintainability in the future.
Therefore, my choice is Option 1, implemented using ResizeObserver.

I will complete the work and submit a PR in the next few days. If you have any better suggestions for my solution, please let me know.

Comment 21

[WhiteMind]

Attachment: Bug 1599297 - Keep addon card header fixed while scrolling. r=mstriemer,rpl

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.