Skip to content

Show a useful icon in SQL tabs - #2153

Merged
mgrojo merged 2 commits into
masterfrom
sql_tab_icons
May 2, 2020
Merged

mgrojo merged 2 commits into
masterfrom
sql_tab_icons

Conversation

@mgrojo

@mgrojo mgrojo commented Mar 2, 2020

Copy link
Copy Markdown
Member

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.

@MKleusberg MKleusberg left a comment

Copy link
Copy Markdown
Member

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 😄

Comment thread src/MainWindow.cpp Outdated
Comment on lines +346 to +347
//QIcon execIcon(":icons/hourglass");
ui->tabSqlAreas->setMinimumSize(ui->tabSqlAreas->iconSize());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

Copy link
Copy Markdown
Member Author

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.

Comment thread src/MainWindow.cpp Outdated
Comment on lines 1207 to 1208
// 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.

@MKleusberg MKleusberg Mar 10, 2020 •

Copy link
Copy Markdown
Member

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 😄

Copy link
Copy Markdown
Member Author

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)
@mgrojo

mgrojo commented Mar 12, 2020

Copy link
Copy Markdown
Member Author

@MKleusberg I've fixed the observations and I propose another icon, which we already had but without use.

@mgrojo
mgrojo merged commit 81b9b5a into master May 2, 2020
@mgrojo

mgrojo commented May 2, 2020

Copy link
Copy Markdown
Member Author

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.

@mgrojo
mgrojo deleted the sql_tab_icons branch May 2, 2020 22:49
@MKleusberg

Copy link
Copy Markdown
Member

Thanks, @mgrojo! And sorry for the delay on my side 😇

@mgrojo

mgrojo commented May 6, 2020

Copy link
Copy Markdown
Member Author

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 😉

@MKleusberg

Copy link
Copy Markdown
Member

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 😄

justinclift pushed a commit that referenced this pull request Jun 6, 2020
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)
@justinclift justinclift added this to the 3.12.0 - Next release milestone Jun 8, 2020
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