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

Inline Tabs for Actor sheets - Pop-Out! workaround #195

Open
wants to merge 2 commits into
base: release-0.2.2
Choose a base branch
from

Conversation

zithith
Copy link
Collaborator

@zithith zithith commented Dec 14, 2024

Type

  • Bug fix
  • Feature
  • Refactor
  • Other (please describe):

Description
Added a setting toggle (as per the item sheet one, but inverted) to make the tabs for actor sheets fall in-line, more like the default foundry behaviour. Required a small re-factor of the navigation parts for the actor sheets so that they appear in a relevant place in the sheet structure.

Related Issue
Closes #194

How Has This Been Tested?
Local machine. Opened sheets with and without the setting toggled to see the change (doesn't re-render already opened sheets).

Screenshots (if applicable)
image
image

Checklist:

  • I have commented on my code, particularly in hard-to-understand areas.
  • My changes do not introduce any new warnings or errors.
  • My PR does not contain any copyrighted works that I do not have permission to use.
  • I have tested my changes on Foundry VTT version: 12.331.

@zithith
Copy link
Collaborator Author

zithith commented Dec 14, 2024

Note the styling is not great... at all...
A) It's a workaround
B) I didn't want to upset the merge conflicts for mango's styling branches more than I had to.

I did make the decision that I left out the labels on the tabs as there are too many of them for the space on the sheet. Though if people think adding them back for accessibility is better, we could look at stacking icon over text?

@zithith zithith self-assigned this Dec 14, 2024
@zithith zithith added ui User interface related issue ux User experience related issue character sheet labels Dec 14, 2024
@zithith zithith linked an issue Dec 14, 2024 that may be closed by this pull request
1 task
@zithith zithith added this to the Release 0.2.2 milestone Dec 14, 2024
Copy link
Collaborator

@MangoFVTT MangoFVTT left a comment

Choose a reason for hiding this comment

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

This is a fine change for 0.2.2, but will need to be discarded and rebuilt for 0.3.0 I suspect, since the CSS layout won't translate properly

Comment on lines +1102 to 1108
"actorSheetInlineTabs": {
"name": "Horizontal Inline Tabs for Actor Sheets",
"hint": "If enabled, actor sheets will use horizontal tabs within the bounds of the sheet, as per foundry default sheets. This setting is meant as a work-around solution for the Pop-Out! module, for those who want to use it."
},
"itemSheetSideTabs": {
"name": "Vertical Side Tabs for Item Sheets",
"hint": "If enabled, item sheets use vertical tabs down the right-hand side, similar to the character sheet, instead of the default in-line horizontal ones."
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should make the setting consistent here where on = vertical and off = horizontal. It doesn't make sense for the 2 settings to do opposite things

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had considered that, but I felt it made more sense to me that the defaults for any optional settings are "off" and that those defaults set it to the way we are suggesting people start.

@zithith
Copy link
Collaborator Author

zithith commented Jan 16, 2025

This is a fine change for 0.2.2, but will need to be discarded and rebuilt for 0.3.0 I suspect, since the CSS layout won't translate properly

Yeah, looking at the handlebars changes for your styling PR as well this seems like it would be almost entirely re-worked. Probably best to just strike it from 0.2.2 so we don't end up reverting functionality, especially as previously mentioned, it doesn't seem to do what people were after (at least according to my testing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
character sheet ui User interface related issue ux User experience related issue
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

Enable support for Pop-out module (workaround)
2 participants