FriendsOfFlarum / links

Manage Flarum primary navigation links
MIT License
37 stars 11 forks source link

feat: add setting to only show icons on small screens #54

Closed SychO9 closed 1 year ago

SychO9 commented 1 year ago

Changes proposed in this pull request: Links can overflow the header of the app, this PR adds a setting to only show icons on smaller screens.

Reviewers should focus on:

Screenshot Screenshot from 2023-02-01 13-03-00

Confirmed

Required changes:

iPurpl3x commented 1 year ago

Hi @SychO9,

I have tried this update and I have encountered an error on one of my instances:

variable @fof-links-show-only-icons-on-mobile is undefined in file /opt/flarum/vendor/fof/links/less/forum.less in forum.less on line 74, column 15

On a clean installation without any other extensions, it seems to work, so what could be the cause of this?... I have checked and there are no other installed extensions that use the registerLessConfigVar() extender method.

Then I also realized that on small mobile devices (phone breakpoint) the icon is also hidden, even though the links are in the burger menu, and there it doesn't make sense to hide the label IMO.

image

Finally, I find the settings name quite confusing, it would be much simpler to name it like this: "hide link label on tablet" (not "on mobile", as mentioned above.

SychO9 commented 1 year ago

For the label and the icons issue: https://github.com/FriendsOfFlarum/links/pull/57 (thanks for the report)

For the first issue, it's hard to tell what could be wrong, it could be a type of extension that overrides certain core components. I would try disabling the most advanced extensions one at a time and see.

iPurpl3x commented 1 year ago

Yeah, I now found the reason and could fix it: I was overriding the flarum.assets.factory. I could extend it instead of overriding it, that fixed the issue of the variable being undefined.