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

Added compatibility for flarum-tag-password extension #49

Merged
merged 7 commits into from
Apr 14, 2024

Conversation

Yippy
Copy link
Contributor

@Yippy Yippy commented Mar 12, 2024

Add support for flarum-tag-passwords, where Admin can lock specific tag with Password and Group Permission, the template would need to prevent loading the stats and last discussion.

Here is an example of it turned on:
image

image

@Yippy
Copy link
Contributor Author

Yippy commented Mar 12, 2024

Fix the background colour for issue [#41] [#45]

image

image

@Yippy
Copy link
Contributor Author

Yippy commented Mar 13, 2024

Fix design when view is very narrow, put spacing to ensure icon is not grouped closely to text.

image

…other themes. Recreate all styling to topNav, and keep sideNav for non-full screen mode.

Fixed compact mobile view mode, where larger screen show sub tags on the right side.
@Yippy
Copy link
Contributor Author

Yippy commented Mar 15, 2024

Fixed: Missing 'Tags' in navigation, with 'Keep the tags page link in the nav sidebar?'
Fixed: Compact mobile mode view design

image

Copy link
Owner

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Hey, thank you so much for these fixes and contributions, really sorry it took so long to get to. Left a few minor comments but otherwise looks good!

@@ -9,10 +9,19 @@ import LastDiscussionWidget from './components/LastDiscussionWidget';
import StatWidget from './components/StatWidget';

function pruneIndexNav(items, func) {
if (app.current.matches(CategoriesPage) && app.forum.attribute('categories.fullPageDesktop')) {
const isTagsVisible = !app.forum.attribute('categories.keep-tags-nav');
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this negated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the tag navigation should not be checked if fullPageDesktop is true, hence why it's removed. This caused the Tag navigation to be shown for the user.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, I agree about the fullPageDesktop change. But you have isTagsVisible being set to the opposite of whether the user wants to keep tags. Shouldn't it be:

isTagsVisible = app.forum.attribute(... keep-tags-nav)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems I've used the wrong attributes, the latest commit is 'categories.keepTagsNav' and fixed the logic

@@ -17,11 +17,18 @@ interface Attrs {
parent: any;
}

interface TagLocked {
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not completely opposed to including features for the tag lock extension here, but would it be possible to add a comment here explaining which extension is adding this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment for reference of Flarum Tag Passwords

@@ -9,10 +9,19 @@ import LastDiscussionWidget from './components/LastDiscussionWidget';
import StatWidget from './components/StatWidget';

function pruneIndexNav(items, func) {
if (app.current.matches(CategoriesPage) && app.forum.attribute('categories.fullPageDesktop')) {
const isTagsVisible = !app.forum.attribute('categories.keep-tags-nav');
Copy link
Owner

Choose a reason for hiding this comment

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

Ah, I agree about the fullPageDesktop change. But you have isTagsVisible being set to the opposite of whether the user wants to keep tags. Shouldn't it be:

isTagsVisible = app.forum.attribute(... keep-tags-nav)?

Copy link
Owner

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Thanks again for putting this together!

@askvortsov1 askvortsov1 merged commit d7156a3 into askvortsov1:master Apr 14, 2024
1 check passed
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.

2 participants