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 a theme to override the form factor of the tabs to be mobile #5225

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

andydotxyz
Copy link
Member

This avoids being forced to respect the singleton device reported from hardware

Opening for discussion about this use for the Size callback avoiding a breaking change or new API requirement for a "theme feature".

Also test to follow.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

This avoids being forced to respect the singleton device reported from hardware
@coveralls
Copy link

Coverage Status

coverage: 59.962% (+0.001%) from 59.961%
when pulling d0421e6 on andydotxyz:feature/simulatemobiletheme
into de361f6 on fyne-io:develop.

@dweymouth
Copy link
Contributor

I think a new API would be much cleaner than overriding the meaning of Size in themes like this

@andydotxyz
Copy link
Member Author

I think a new API would be much cleaner than overriding the meaning of Size in themes like this

What would such an API be like? Would it be an optional extension? The part that I'm not sure about is that some sort of "config" section could be all sorts of types int/bool/float/string...

@dweymouth
Copy link
Contributor

I think a new API would be much cleaner than overriding the meaning of Size in themes like this

What would such an API be like? Would it be an optional extension? The part that I'm not sure about is that some sort of "config" section could be all sorts of types int/bool/float/string...

I guess I need to know more about the planned use cases. If this is just about the tabs widget, I would just add a new property API to AppTabs and DocTabs to control this, similar to the Orientation property of RadioGroup and CheckGroup. If it's about overriding device settings in general, maybe a new set of APIs related to device and not the theme? If it's about general, open-ended settings that are tied to the theme, then some more API design may be needed (and more use case discovery to guide the API design)

@andydotxyz
Copy link
Member Author

It's not the same thing as orientation, as the tabs respond differently on different platforms rather than just switch configuration.
If the options are exposed separately then something overriding the context becomes tied to the render implementation for AppTabs.

I wonder if it could instead be an api on ThemeOverride if being on theme isn't looking like a good approach?
It would probably still need to be passed internally through theme or a whole new cache that indexes on theme. I was trying to keep it simple by passing it in Size but I'll keep thinking.

@andydotxyz andydotxyz merged commit d0421e6 into fyne-io:develop Nov 5, 2024
12 checks passed
@andydotxyz
Copy link
Member Author

This was merged by mistake somehow. The commit was reverted on develop and re-applied on this branch so the conversation can continue where we left off.

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