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

Allow comment instead of blank title #762

Merged
merged 1 commit into from Jan 14, 2020
Merged

Conversation

@geekwright
Copy link
Contributor

geekwright commented Jan 14, 2020

Suppress display of block titles starting with // .

The display of a block title can be suppressed by leaving the title field blank, but this makes it difficult to identify blocks when editing in the admin section.

This change allows "commenting" out a title by starting it with a slash-slash-space sequence. Blocks with a title starting with this sequence will be displayed as if their title was blank.

Suppress display of block titles starting with `// `.

The display of a block title can be suppressed by leaving the
title field blank, but this makes it difficult to identify
blocks when editing in the admin section.

This change allows "commenting" out a title by starting it with
a slash-slash-space sequence. Blocks with a title starting with
this sequence will be displayed as if their title was blank.
@mambax7

This comment has been minimized.

Copy link
Collaborator

mambax7 commented Jan 14, 2020

Would underscore be better? I would make it more consistent with previous techniques that Trabis was using:
https://xoops.org/modules/news/article.php?storyid=4993

@alain01

This comment has been minimized.

Copy link

alain01 commented Jan 14, 2020

The "// " (slash, slash, space) is a comment indicator in many languages

Yes, I think it's a good choice !
But Michael, i'm ok with "_" too.
The effective point is to code it in the core, not in the theme !

This would allow switching themes without resulting in a bunch of "@ - " block titles

@GregMage

This comment has been minimized.

Copy link
Contributor

GregMage commented Jan 14, 2020

I find that the use of "//" makes the most sense!

You must add information in the language define to explain this option. Otherwise no one will use it!

@mambax7 mambax7 merged commit 84a0ae9 into XOOPS:master Jan 14, 2020
1 check passed
1 check passed
Scrutinizer Analysis: 1 updated code elements – Tests: passed
Details
@alain01

This comment has been minimized.

Copy link

alain01 commented Jan 14, 2020

Yeaaaaah !
Merged and yet tested and operationnel on my site !
Great !
Thank you !

@ggoffy

This comment has been minimized.

Copy link
Contributor

ggoffy commented Jan 14, 2020

hi.
my opinion: yes, it works and it is a simple solution. but for daily use of xoops by "non-experts" it would be nicer if we have in the block administration a new option e.g. "show block title: yes/no" because this would understand everyone ;)

@alain01

This comment has been minimized.

Copy link

alain01 commented Jan 14, 2020

Sure, ggoggy ! I have proposed this solution here : https://xoops.org/modules/newbb/viewtopic.php?post_id=364789
But need to add a new field in database + some cosmetic lines in the managment blocks page.

@geekwright geekwright deleted the geekwright:blocktitle branch Jan 14, 2020
@geekwright

This comment has been minimized.

Copy link
Contributor Author

geekwright commented Jan 14, 2020

I agree that there could be better solutions, but there is a full rework of the blocks system slated in 2.6. That is where new feature requests will be entertained.

This was a simple fix for a troublesome issue, You can now see more detail for blocks with suppressed headers, and easily recognize such blocks. #763 adds some documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.