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

Support Widget display on Categories Page and Fix Text Color using textContrastClass #50

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Yippy
Copy link
Contributor

@Yippy Yippy commented Apr 29, 2024

Added new settings to enable or disable different sections of widgets
Categories Page Support for widgets

Fix #25 Using textContrastClass and applying filter for sub heading contrast
Text Color Fix

…black font color depending on the color of the tag and amend the style of the sub heading with filter brightness for different color contrast
@Yippy
Copy link
Contributor Author

Yippy commented Apr 29, 2024

For afrux left sidebar top and left sidebar bottom to display correctly within Categories page, the PR afrux/forum-widgets-core#17 needs to be merged. But header, footer, and right widget will display correctly without the PR.

@Yippy
Copy link
Contributor Author

Yippy commented Apr 29, 2024

Working on fixing dark mode, will remove the workaround as this causes issues. Will also remove filter brightness for just applying styling.

… match with forum.js

Added 2 style theming for both parent and child tags
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 for the PR!

->serializeToForum('categories.childBareIcon', 'askvortsov-categories.child-bare-icon', 'boolval', true),
->serializeToForum('categories.childBareIcon', 'askvortsov-categories.child-bare-icon', 'boolval', true)
->serializeToForum('categories.widgetHeader', 'askvortsov-categories.widget-header', 'boolval', false)
->serializeToForum('categories.widgetRight', 'askvortsov-categories.widget-right', 'boolval', false)
Copy link
Owner

Choose a reason for hiding this comment

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

Why not default these to true? If someone has installed a widget extension, presumably they want widgets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not entirely, I though it was better to turn them off by default with the original categories page. No one seems to have complained about not having widgets on the page. The original didn't have header, footer or right side widget. Only there was a bug for allowing the widget to show on the left sidebar, which I accidently fixed. This PR was to make it almost like before, but with options

js/src/forum/components/CategoriesPage.tsx Outdated Show resolved Hide resolved
})}
</ol>

{cloud.length ? <div className="TagCloud">{cloud.map((tag) => [tagLabel(tag, { link: true }), ' '])}</div> : ''}
</div>,
50
);

// Only check for widget for the right if enable in the settings
if (app.forum.attribute('categories.widgetRight')) {
Copy link
Owner

Choose a reason for hiding this comment

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

Is there no way for the widgets plugin to provide an API for reliably getting it's widgets? Or for it to extend this plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well the plugin doesn't have option to hide or show widget on pages, I've provided a toggle to give admin the choice of showing what is needed for their categories page. Some people have them as their main page, while I don't have it as the mainpage. But the widget is useful else where on the website.

items.add('icon', <span className="TagCategory-icon">{this.iconItems().toArray()}</span>, 100);
const children = this.isChild ? [] : sortTags(this.tag.children() || []);
let style: Record<string, unknown> = {};
if (this.isChild) {
Copy link
Owner

Choose a reason for hiding this comment

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

This "style building" code seems to be repeated; could you extract it into a helper function?

@@ -12,16 +12,30 @@ askvortsov-categories:
parent_remove_stats: Hide stats for top-level tags?
parent_remove_last_discussion: Hide most recent discussions for top-level tags?
small_forum_optimized: Optimize for small forums?
enable_primary_tag_color: Enable Primary Tag Color?
Copy link
Owner

Choose a reason for hiding this comment

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

Why don't we always want to enable tag colors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've shown examples of the theme, which can give admin the choice of having it all minimalistic with the dark and light support.

@Yippy
Copy link
Contributor Author

Yippy commented May 1, 2024

Theme Choices Example
image
example of primary tag colors enabled

image
example of primary tag colors disabled gif

image
example of primary tag colors just parent enabled gif

image
example of primary tag colors just child enabled gif

Please note that Child Tag 4 has no color assigned, it's left as '[empty]' for testing, I will work on @askvortsov1 concerns. It does need refactoring in some areas. But I'm trying to work on datlechin/flarum-tag-passwords#11 to hide protected discussion and fix the discussion protection within this first.

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.

Text color
2 participants