Skip to content
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

Overhaul of the user panel menu #4603

Merged
merged 26 commits into from Dec 9, 2021
Merged

Overhaul of the user panel menu #4603

merged 26 commits into from Dec 9, 2021

Conversation

dtdesign
Copy link
Member

@dtdesign dtdesign commented Dec 8, 2021

The menu has been rebuilt from the ground up with simplicity, consistent styling and a11y in mind. It introduces a set of new style variables to avoid the pitfall of deriving colors from different elements just because the happen to fit in nicely in the default style.

This change is the first step towards the new mobile menu which will be built on top of this. For now the conversation menu has been left unchanged to serve as a blueprint for the backwards compatibility layer.

image

image

Favico.js was used exclusively for the notification badge in the favicon, but was replaced with a lightweight implementation.

The user panel items cannot be properly mapped to the new implementations, if a custom version of `pageHeaderUser.tpl` is being used, then they are screwed anyway.
```
cp node_modules/tabbable/dist/index.umd.min.js wcfsetup/install/files/js/3rdParty/focus-trap/
cp node_modules/tabbable/dist/index.umd.min.js.map wcfsetup/install/files/js/3rdParty/focus-trap/
```

```
cp node_modules/focus-trap/dist/focus-trap.umd.min.js wcfsetup/install/files/js/3rdParty/focus-trap/
cp node_modules/focus-trap/dist/focus-trap.umd.min.js.map wcfsetup/install/files/js/3rdParty/focus-trap/
```
It doesn't play nicely with the UI/UX concept
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eyeballed everything, except for pageHeaderUser.tpl. The changes are just too large in there.

@dtdesign
Copy link
Member Author

dtdesign commented Dec 8, 2021

The commit c9e8db8 might not be immediately obvious, but this is pretty much what the previous implementation does by slapping top: 50px !important on the element using CSS. I'm taking the extra steps here to make it possible for style creators to overwrite the position with absolute and having the computed value match. This is useful when the header is not fixed and was horribly awkward to work around.

@dtdesign dtdesign marked this pull request as ready for review December 8, 2021 20:49
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I'm qualified to tell this LGTM now. I did not perform a test, this was a review purely based on the diff.

@dtdesign dtdesign merged commit d3e6c74 into master Dec 9, 2021
@dtdesign dtdesign deleted the user-panel-v2 branch December 9, 2021 13:59
TimWolla added a commit that referenced this pull request Dec 10, 2021
The one in the frontend has already been removed in #4603.
372f863 is partially related as well.
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.

None yet

2 participants