Skip to content

delete internal/local shrink_title function in Adjustable.Containers + add position as option for setTitle#5493

Merged
vadi2 merged 4 commits intoMudlet:developmentfrom
Edru2:adjustable-container-delete-shrink-title
Oct 11, 2021
Merged

delete internal/local shrink_title function in Adjustable.Containers + add position as option for setTitle#5493
vadi2 merged 4 commits intoMudlet:developmentfrom
Edru2:adjustable-container-delete-shrink-title

Conversation

@Edru2
Copy link
Copy Markdown
Member

@Edru2 Edru2 commented Oct 7, 2021

Brief overview of PR changes/additions

Gets rid of internal shrink_title function.
Adds the format/position parameter for Adjustable.Container:setTitle(title,colour, format)
for example:

myContainer:setTitle("TestTitle","yellow","c")

Also adds Adjustable.Container:resetTitle() to set the title to the default one.

Motivation for adding to Mudlet

shrink_title caused weird issues for example #5318 and it never worked as expected.
I saw many screenshot of adjustable container where it simply didn't do what is should have (delete the last letters of the text
and replace it with ... to prevent text behind the minimize and close buttons)
fix #5318

Other info (issues closed, discussion etc)

it will also allow more flexibility for advanced users which will be able to override the setTitle function if wanted.

Release post highlight

-Adjustable container won't reveal html entities in title any-more if shrinking container
-Adjustable container setTitle functions gets a format parameter, title can be centered now.
-Adjustable container new function resetTitle to set the title to the default one

@Edru2 Edru2 requested a review from a team October 7, 2021 20:53
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Oct 7, 2021

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Oct 10, 2021

Could the buttons have a background colour, so the text doesn't go "under" them?

Screen.recording.2021-10-10.7.59.34.AM.mp4

@Edru2
Copy link
Copy Markdown
Member Author

Edru2 commented Oct 10, 2021

Could the buttons have a background colour, so the text doesn't go "under" them?
Screen.recording.2021-10-10.7.59.34.AM.mp4

The red colour of the buttons is their background colour. It would be possible to put the title on it's own (transparent and clickthrough) label, but I'm not sure what other consequences (in regards of backward compatibility) that would have.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Oct 10, 2021

Could we put the red buttons on a square black label, to the text just stops at one point instead of going under the buttons?

@Edru2
Copy link
Copy Markdown
Member Author

Edru2 commented Oct 10, 2021

Could we put the red buttons on a square black label, to the text just stops at one point instead of going under the buttons?

I tested it and it doesn't work well as it will cover the border and if someone changes the background to something non-black it will stay black (changing that squares background colour when changing the containers background colour also doesn't solve the problem as there can also be images as background)

Putting the text on a transparent/clickthrough label works well, but it will duplicate the text on Adjustable.Tabwindow tabs until I release a fix for that there because Adjustable.Tabwindows use a custom overridden echo function for the text on the tabs (which are Adjustable.Containers)

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

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

ok 👍

in the future, could you split out bugfixing & new functionality into two separate PRs? It's OK for the first time here

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.

Shrinking Adjustable size shouldn't reveal HTML entities

2 participants