Transform help popup into a pocket menu

[GuillaumeGomez]

Just like we moved the settings menu into a "pocket menu", it's doing the same to the help popup.

You can test it here and here is a screenshot:

r? @jsha

[rust-highfive]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd,@jsha

[jsha]

Why is it so wide? This is wider than the current help menu, and wider than the settings menu. It seems like keeping it similar width as the settings menu (it doesn't have to be exact) would increase harmony between the two.

[GuillaumeGomez]

Making it the same width would require a lot of height so instead I limited its width to 600px. I updated the screenshot and the demo so you can take a look to see if it's more acceptable this way. :)

[GuillaumeGomez]

Fixed eslint errors.

[notriddle]

I don't think it makes sense for borders inside the help popup to be darker than the borders around it.

I also notice, in Firefox, that I can't select text to copy from it. That's not good.

[GuillaumeGomez]

Nice, great catch! I'll fix it tomorrow. :)

[GuillaumeGomez]

I updated the demo. The blur and style are now correctly handled.

[GuillaumeGomez]

I also extended the GUI test to ensure that all borders have the same color.

[notriddle]

How to get both Settings and Help opened at once (at least in Firefox)

1. First, click the Help button.
2. Press Ctrl+L, making the address bar active. Alternatively, click it.
3. Click the Settings button.
4. Help and Settings are now both open at once, in violation of accepted UX guidelines for modal views like popovers.

How to fix it

Instead of solving this in an ad-hoc way by tying Help and Settings close together, I recommend some principled code reuse between these two popovers.

1. Create a global, mutable variable in main.js for the CURRENTLY_OPENED_POPOVER. Set it to null initially.

2. Instead of having two different functions for hiding the Help and Settings, write one function to hideCurrentPopover. Something like this:

   function hideCurrentPopover() {
       if (!CURRENTLY_OPENED_POPOVER) { return; }
       CURRENTLY_OPENED_POPOVER.style.display = 'none';
       CURRENTLY_OPENED_POPOVER = null;
   }
   

3. When opening one of these two popovers, assign the popover DIV to the variable. You might even write a single function that just takes care of hiding and showing a popover:

   function togglePopoverByID(id) {
       const already_open = CURRENTLY_OPENED_POPOVER && CURRENTLY_OPENED_POPOVER.id === id;
       hideCurrentPopover();
       if (!already_open) {
           CURRENTLY_OPENED_POPOVER = document.getElementByID(id);
           CURRENTLY_OPENED_POPOVER.style.display = 'block';
       }
   }
   

4. Get rid of most of the #help-button and #settings-button selectors in CSS. Use classes instead.

[GuillaumeGomez]

It would be much simpler to just hide the other popover elements. Good idea! That will allow me to remove a function too.

[GuillaumeGomez]

So instead of setting a JS global variable, I simply hide all popovers before showing a new one. Much simpler this way. :)

As for the CSS, what would you simplify?

[notriddle]

This is probably going to be the last set of critiques. A lot of rustdoc's CSS seems overly complicated, but doing a comprehensive cleanup should be a separate PR.

I think the way you wrote the window.hidePopoverMenus function is good. Better than using a global variable. Thanks!

[notriddle]

This selector is pretty complicated. Could it be a utility class instead?

[GuillaumeGomez]

Just in case: it's not one selector but three different selectors on which we apply the same rules. If it was what you understood, I'm not sure to understand how simpler it could be though...

[GuillaumeGomez]

This is probably going to be the last set of critiques. A lot of rustdoc's CSS seems overly complicated, but doing a comprehensive cleanup should be a separate PR.

You're right, but we're maybe not thinking about the same thing. 😆 I have two cleanups in mind:

• I think some rules in rustdoc are not used anymore, I need to check that at some point.
• The themes could be simplified a lot by setting CSS variables instead of reimplementing the rules. I plan to do it as well. I added a reminder to not forget once again.

[GuillaumeGomez]

I simplified the CSS rule as @notriddle suggested.