Skip to content

Conversation

akshatnitd
Copy link
Member

@djmgit Please review :)


self.current_line_style = StringVar(top)
self.current_line_style.set('-')
self.line_styles = {'-', '--', '-.', ':'}
Copy link
Member

Choose a reason for hiding this comment

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

This shows only symbols in the drop down which looks a bit weird. Please show style names on drop down. Like solid line, dashed, dot dashed. Symbols should be present in bracket.

self.dropdown_menu_line_style = OptionMenu(top, self.current_line_style,
*self.line_styles, command=self.dropdown_changed_line_style)
self.dropdown_menu_line_style.pack(side='top', anchor='w')
self.dropdown_menu_line_style.place(relx=0.78, rely=0.60, height=18, width=100)
Copy link
Member

Choose a reason for hiding this comment

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

The position of the drop down is a bit too down. Please move it up a little.

self.dropdown_menu_color.configure(fg=_fgcolorlight)
self.dropdown_menu_color['menu'].configure(activebackground=_activebgcolordark)
self.dropdown_menu_color['menu'].configure(background=_bgcolorlight)
self.dropdown_menu_color['menu'].configure(fg=_fgcolorlight)
Copy link
Member

Choose a reason for hiding this comment

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

Dropdown does not change color on changing theme. Please look into this.

Copy link
Member

@djmgit djmgit left a comment

Choose a reason for hiding this comment

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

Hi, please look into my comments inline.

Copy link
Member

@djmgit djmgit left a comment

Choose a reason for hiding this comment

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

Please see my comments inline.

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.

2 participants