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

Add support for md_thumbnail & md_marquee #634

Merged
merged 1 commit into from Mar 20, 2020

Conversation

supersquared
Copy link

  • Added support for md_thumbnail in VideoGameListView
  • Added support for md_thumbnail & md_marquee in DetailedGameListView

@joolswills
Copy link
Member

Anyone have any issues with these changes?

@supersquared
Copy link
Author

I don't know if I can answer since I am the person who made the changes, but I'm using it since almost one month and half without any issue (many games with and without md_thumbnail and md_marquee)

@pjft
Copy link
Collaborator

pjft commented Mar 18, 2020

Read the code and have a question about the behavior for themes that don't specify the thumbnail - even if the gamelist.xml might have one for the games, as a result of scraping.

Other than that, no objections. Sorry for the delay.

@supersquared
Copy link
Author

Just committed the changes to have thumbnail off the screen by default. is that ok ?

@supersquared
Copy link
Author

@jrassa how do I squash the commits?

@jrassa
Copy link
Collaborator

jrassa commented Mar 19, 2020

@supersquared you need to do a rebase. Here is a pretty good tutorial. Let me know if you need any additional help.

@fabricecaruso
Copy link

You never call setVisible(true), so If you do it that way, theme designers will have to add true in the theme for the elements to be visible :

It should not be required
It's not required for other components. Why should it be diffent ?
If a theme designer adds a md_thumbnail element, it should be visible by default.

Maybe add something like that :

mThumbnail.setVisible(theme->getElement(getName(), "md_thumbnail", "image")); // <- THIS
mThumbnail.applyTheme(theme, getName(), "md_thumbnail", ALL ^ (PATH));

To squash 6 commits do that :
git rebase -i HEAD~6
keep the first item with "pick", replace the others with "s" ( or squash ) keyword & save.
then when complete, force push with
git push -f

- Added support for md_thumbnail in VideoGameListView
- Added support for md_thumbnail & md_marquee in DetailedGameListView
@jrassa
Copy link
Collaborator

jrassa commented Mar 19, 2020

@fabricecaruso There shouldn't be a need to explicitly call setVisible(true). This is already handled in GuiComponent:
https://github.com/RetroPie/EmulationStation/blob/master/es-core/src/GuiComponent.cpp#L440

When applying any other theme properties, visible will be set to true unless explicitly set to false. This was tested and verified when visible was added.

@supersquared
Copy link
Author

Squashed! @fabricecaruso @jrassa thank you for the help on squashing ;)

@fabricecaruso
Copy link

already handled in GuiComponent:

You're absolutely right, sorry. Everything's fine.

@jrassa jrassa merged commit 4eafb87 into RetroPie:master Mar 20, 2020
Yaoh pushed a commit to Yaoh/EmulationStation that referenced this pull request Jul 13, 2020
Add support for md_thumbnail & md_marquee
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.

None yet

5 participants