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

update siteinfo bloc and 1 question #1302

Merged
merged 5 commits into from Mar 5, 2023
Merged

update siteinfo bloc and 1 question #1302

merged 5 commits into from Mar 5, 2023

Conversation

alain01
Copy link
Contributor

@alain01 alain01 commented Jan 29, 2023

The test of $block.showgroups should be before the table tag. Why ?
Cause if $block.showgroups == false, the result is a useless empty table in the code.

I have a question on another point :
Why test

        <!-- start group loop -->
        <{foreach item=group from=$block.groups}>

?
Cause there is only the webmasters group displayed, no other group in this bloc.
So, I think this test is useless, I think we should pick up the group webmasters, no need to use a foreach for that.
No ?

The test  of $block.showgroups should be before the table tag.
Why ?
Cause if $block.showgroups == false, the result is a useless table in the code.

I have a question on another point :
Why test "        <!-- start group loop -->
        <{foreach item=group from=$block.groups}>
" ?
Cause there is only the webmasters group display, no other group in this bloc.
So, I think this test is useless, I think we should pick yp the group webmasters, no need to use a foreach for that.
No ?
@mambax7
Copy link
Collaborator

mambax7 commented Jan 29, 2023

Each block can have multiple groups permitted to see it, incl. the Site Info block, so it's not only the Webmaster. You can easily share the Siteinfo block with Anonymous users, and you can also create new groups and add them there:

image

@GregMage
Copy link
Contributor

That's not it Michael. Alain talks about the display of information.
To answer your question Alain, there can be several administrator groups! If you make a new group in XOOPS and you give it only one system administration rights (for example "Avatars") then this group is an admin group and it appears in the block. I didn't know it worked like that but after a short search here is the proof:
image

@alain01
Copy link
Contributor Author

alain01 commented Jan 29, 2023

Yes, you are right, Greg and thank you for the explanation for Michael.
Michael, it's not the group to display the bloc.
Ok Greg, we could have more admins groups like administration rights (for example "Avatars") !

So, I get an answer for my question ! Thank you Greg.

And the modification for the test $block.showgroups, thank you for this 1st aprobation

@GregMage
Copy link
Contributor

GregMage commented Jan 30, 2023

Alain,
Can you make the changes also on these files:

  • htdocs\themes\xbootstrap\modules\system\blocks\system_block_siteinfo.tpl
  • htdocs\modules\system\templates\blocks\system_block_siteinfo.tpl

@alain01
Copy link
Contributor Author

alain01 commented Jan 30, 2023

ok, wait, I add other files in this PR

Added a test to the <thead> line cause bad code in source with an empty < th > </ th > for the elements after the first element in each group
same as in the xSwatch4 theme
same as in the xSwatch4 theme
@GregMage
Copy link
Contributor

Great job, thank you Alain!

@GregMage
Copy link
Contributor

We can validate this PR!

@mambax7 mambax7 merged commit 3a64988 into XOOPS:master Mar 5, 2023
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

4 participants