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

Improved the Titles tab on the Graphs subdialog #8337

Merged
merged 20 commits into from Jun 2, 2023

Conversation

Vitalis95
Copy link
Contributor

Fixes #8299
@rdstern @lloyddewit , this PR improves the titles tabs in Plot sub-dialog

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 this is looking very promising. Just what I was hoping for.

a) Could you also tempt the Caption to be multi-line as we do in the PICSA climatic sub-dialogue.
b) Could you also add the tooltip from there that explains that \n can be used to provide a multiline caption.

c) A sutprise!
image

Here is an example, that gave me a small surprise. The x and y axis sizes seemed to be affected by the changes in the font size for the title, or subtitle. (I would like also to be able to choose their size, but perhaps not there!

d) There is something odd with your code and the defaults. I get a very different size for the plot title and sub-title when I have the defaults of 20 and 15, compared with 20.1 and 15.1. Could thise defaults be the actual ones as well as not linking to the x axis and y axis sizes.

e) Could the Tag and New Legend Title have checkboxes, default unchecked, The text is only visible when checked. Maybe the tag and also perhaps the Legend needs the Size too, and I am not sure the Legend title works yet?

@Vitalis95
Copy link
Contributor Author

@rdstern have a look at it

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 this is looking good now.
a) When I first unchecked the tag it was as follows:
image

But when I returned to the sub-dialog the size of the tag had changed to 8?
b) In the resulting ggplot I am not sure all the components are the size givewn - sometimes it is given, and sometimes not, so it presumably uses the default. I suggest anytime a component is used (i.e. not null) then you explicitely put the size into the R command.
c) I am not sure what the Fill is doing for the new legend? Doesn't that provide a colour? If so, then the type of control is not correct.

@Vitalis95
Copy link
Contributor Author

@rdstern , the New Legend and Fill modifies the legend title for the color and the legend title for the fill aesthetics, they do not control the appearance or placement of the legend itself as stated by the function.

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 here is the graph after input of a title and sub-title and caption.
I think it is probably a single error, but it looks as though the y-axis and x-axis and legends title are the same size as the main title?

@Vitalis95
Copy link
Contributor Author

@rdstern , have a look at it

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 this all seems to work well now.

My only problem is the funny 2 parameters for the legend. This entry seems to indicate I am not the only one to be confused.

I suggest you just have the Legend title control there - and still use the fill parameter in the R code that you generate.
Then we can merge.

@Vitalis95
Copy link
Contributor Author

@rdstern , check it now

rdstern
rdstern previously approved these changes May 25, 2023
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 great. @lloyddewit over to you

instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
instat/sdgPlots.vb Outdated Show resolved Hide resolved
Vitalis95 and others added 10 commits May 29, 2023 13:01
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
@lloyddewit
Copy link
Contributor

@Vitalis95 Thank you for the changes, I think there are still 2 unresolved comments above, please could you check?
thanks

@Vitalis95
Copy link
Contributor Author

@lloyddewit , it is fine now.

@lloyddewit
Copy link
Contributor

@rdstern There are some small changes since your last approval. If you can test/approve again, then we can merge, thanks

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

Great. Thanks

@lloyddewit lloyddewit changed the title Improving the Titles tab on the graphs sub-dialogue Improved the Titles tab on the Graphs subdialog Jun 2, 2023
@lloyddewit lloyddewit merged commit ff681b2 into IDEMSInternational:master Jun 2, 2023
2 checks passed
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.

Improve the Titles tab on the graphs sub-dialogue
3 participants