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

DolphinWX: Disable 'maker' column hiding #3687

Merged
merged 1 commit into from
Apr 21, 2016

Conversation

rukai
Copy link
Contributor

@rukai rukai commented Feb 29, 2016

This was done because showing a column was broken:
Showing a column repopulates the column with no regard for the sorted
order. This results in a seemingly random order.
(actually the order of m_ISO_FILES)

Review on Reviewable

@BhaaLseN
Copy link
Member

Does this work correctly if you sort by a(ny) column and then hide a(ny other) column using the menu options? Or does this trigger a reload of some sorts?
In case it works correctly: what keeps use from using the same technique here?
On the other hand: I don't mind a horizontal scrollbar if the window is too small, so dropping this functionality is fine too.

@rukai
Copy link
Contributor Author

rukai commented Feb 29, 2016

Hiding columns via the menu option triggers a reload.

I have tried to get the functionality working but decided it wasnt worth pursuing it more.
If anyone really wanted to get this functionality working the best way I could see would be sorting m_ISOFiles to stay in sync with the table row order.


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@RisingFog
Copy link
Member

:lgtm:


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@delroth delroth added this to the Dolphin Release 5.0 milestone Mar 26, 2016
@RisingFog
Copy link
Member

Anything blocking this from merge?

@JMC47
Copy link
Contributor

JMC47 commented Apr 13, 2016

LGTM

@mimimi085181
Copy link
Contributor

mimimi085181 commented Apr 14, 2016

Shouldn't this pr also remove the ShowColumn function, as well as the entries the 2 not used functions in the .h file?

Edit: Ok, i see it does remove the show function as well. But i think the entries for the functions should also be removed from GameListCrt.h.

This was done because showing a column was broken:
Showing a column repopulates the column with no regard for the sorted
order. This results in a seemingly random order.
(actually the order of m_ISO_FILES)
@mimimi085181
Copy link
Contributor

Thanks for removing the functions from the .h files as well.

I've tested this, and there don't seem to be any issues with it. The maker column always has the correct entries now, even if i try to set the width the 0 by resizing it with the mouse.

There's still one issue, but it's not related to the pr or the issue it tries to fix(https://bugs.dolphin-emu.org/issues/9437):
Whenever i reduce the width of the window, it switches back and forth between having a vertical scrollbar. That scrollbar is completely useless, because there's nothign to scroll, and you can't even scroll 1 pixel.

LGTM

@lioncash
Copy link
Member

Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@lioncash lioncash merged commit bdb9da2 into dolphin-emu:master Apr 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants