Skip to content

Allow profile tab bars to be moved around#4790

Closed
vadi2 wants to merge 2 commits intoMudlet:developmentfrom
vadi2:reenable-moverable-tab-bars
Closed

Allow profile tab bars to be moved around#4790
vadi2 wants to merge 2 commits intoMudlet:developmentfrom
vadi2:reenable-moverable-tab-bars

Conversation

@vadi2
Copy link
Copy Markdown
Member

@vadi2 vadi2 commented Feb 12, 2021

Brief overview of PR changes/additions

Allow profile tab bars to be moved around

Peek 2021-02-12 20-20

Motivation for adding to Mudlet

For player convenience

Other info (issues closed, discussion etc)

The comment that was added was incorrect - the underlying console does move around OK.

@vadi2 vadi2 requested a review from a team as a code owner February 12, 2021 19:06
@vadi2 vadi2 requested a review from a team February 12, 2021 19:06
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Feb 12, 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.


void dlgMapper::updateInfoContributors()
{
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wait but why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Accidental commit to work around the crash

mpTabBar->setTabsClosable(true);
mpTabBar->setAutoHide(true);
connect(mpTabBar, &QTabBar::tabCloseRequested, this, &mudlet::slot_close_profile_requested);
// TODO: Do not enable this option currently - although the user can drag
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I remember vivid discussion in the original pr hence the temporary deactivation here. The pr number eludes me. May be worthwhile to review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Seems to work OK in testing

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See new gif

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please click multiplay button to see 2 or 3 profiles simultaneously, then move the headers around

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤦‍♂️ Um, that was why I left the comment there...

@vadi2 vadi2 closed this Feb 12, 2021
@vadi2
Copy link
Copy Markdown
Member Author

vadi2 commented Feb 12, 2021

This does not work with multiview.

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.

3 participants