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

Fix incorrect version in Finder's Info view #658

Merged
merged 1 commit into from Jul 15, 2016

Conversation

revolter
Copy link
Member

Related to #551.

Fix similar to QupZilla/qupzilla#1486.

@justinclift
Copy link
Member

Awesome. 😄

@justinclift
Copy link
Member

Note - Just waiting for the Travis CI build to ok this, Just In Case. 😉

(and no, not expecting it to fail)

@vtronko
Copy link
Member

vtronko commented Jul 15, 2016

Why don't just push it, since diff is straightforward and not ambiguous(thus no discussion needed) 😄

@justinclift justinclift merged commit 4c0f680 into sqlitebrowser:master Jul 15, 2016
@justinclift
Copy link
Member

Yeah, ok. You convinced me. 😀

@revolter
Copy link
Member Author

Because I don't trust myself yet to directly push changes without PR. Don't want to make mistakes like #655 x_x

@justinclift
Copy link
Member

Absolutely nothing wrong in being cautious when you feel like it. 😄

@vtronko
Copy link
Member

vtronko commented Jul 15, 2016

Yet, just IMO huge amount of merge commits pollutes the log.
Good work on issues btw 👍

@justinclift
Copy link
Member

justinclift commented Jul 15, 2016

Lots of merge commits do look pretty nifty in the Network graph though. Whooo! 😁

@revolter
Copy link
Member Author

@innermous, Ok, I'll push trivial changes directly. And, thanks 😄

@vtronko
Copy link
Member

vtronko commented Jul 15, 2016

It's so miserable glitch, that I won't even open another issue for that.
In this about dialog in version label you can put cursor blink in any random position. It feels a bit awkward.
@justinclift should this be left just as is?

@justinclift
Copy link
Member

Up to you. If you want to adjust it, go for it. 😄

@revolter
Copy link
Member Author

@innermous, I don't think it's a good idea. It actually was intentional as seen in #222. Apple does that too:

screen shot 2016-07-16 at 19 55 26

so you can easily copy and paste the version when asked for. I agree it should leave the cursor there, blinking, but disabling selecting is not a good idea.

Also, being able to select the text "Version" is not great also, but it can't be easily done in the current state, as it's just a big string.

@justinclift
Copy link
Member

Hmmm, yeah. I'd completely forgotten about #222. 😵

Just tried out 109ead9 here, and the version text isn't selectable at all with that.

So, it sounds like the optimal approach would be to have the text be selectable, but without any kind of blinking cursor. Is that achievable?

@revolter
Copy link
Member Author

@justinclift, Done 😄

@justinclift
Copy link
Member

Thanks @revolter, that's great!

Just tried it here, and yep it's selectable & no cursor seems present. All good. 😄

@revolter
Copy link
Member Author

My first "direct" commit also 😄

@vtronko
Copy link
Member

vtronko commented Jul 16, 2016

@revolter alrighty then. I forgot about copy-paste use case.

@revolter
Copy link
Member Author

No problem

@revolter revolter deleted the hotfix/osx-version branch May 30, 2018 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants