-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Show a useful icon in SQL tabs #2153
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than these two minor points I don't see any problem with this. We might want to add two extra icons for this purpose though because I think if I didn't know about the meaning of these icons I wouldn't have guessed it correctly. On the other hand I couldn't find better icons either 😉 So feel free to merge this with the current icons - fixing a bug is more important than good looking icons I guess 😄
src/MainWindow.cpp
Outdated
//QIcon execIcon(":icons/hourglass"); | ||
ui->tabSqlAreas->setMinimumSize(ui->tabSqlAreas->iconSize()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, sorry. A test leftover.
src/MainWindow.cpp
Outdated
// NOTE It's a bit hack-ish but we don't use this icon just as a signal to the user but also check for it in various places to check whether a | ||
// specific SQL tab is currently running a query or not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove that comment, now that it is outdated? Or better yet, update it to explain that the tab data value means 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course. I'll update.
The icon shows whether the tab is linked to a project file (open_sql icon) or to a file in disk (document_open icon). This has a lateral effect of fixing this, since the tab always has an icon and the height of the tab bar never changes: #2073 (comment)
@MKleusberg I've fixed the observations and I propose another icon, which we already had but without use. |
I've merged this to master, since the observations had been fixed and now we have a 3.12.x version and the decision to merge it to that release can be taken later. |
Thanks, @mgrojo! And sorry for the delay on my side 😇 |
Don't worry! You had already reviewed it. Should we consider it as candidate for 3.12.x? I fear spartan users who never seem to have enough space, though 😉 |
Yeah, let's include it in 3.12.x because besides being more useful it somewhat fixes the multithreading code we introduce in that release 😄 |
The icon shows whether the tab is linked to a project file (open_sql icon) or to a file in disk (document_open icon). This has a lateral effect of fixing this, since the tab always has an icon and the height of the tab bar never changes: #2073 (comment)
The icon shows whether the tab is linked to a project file (tab icon) or
to a file in disk (document_open icon).
This has a lateral effect of fixing this, since the tab always has an icon
and the height of the tab bar never changes:
#2073 (comment)
I had this change in my local repository and don't want to merge it so close to the release without some feedback. The code changes are minimal, but I'm not sure about whether we want icons there, or what icons should be used. For #2073 there is probably other solution, but I was thinking some time ago that it would be nice to have a visual way to know whether the tab is saved in a file or in the project file. At some time we might also want to have SQL files saved in the project file as a reference to an external file, and not as a saved text embedded in the project.