sugarlabs / chat

The Sugar chat activity
GNU General Public License v2.0
5 stars 24 forks source link

Add clear button to the entry widget #37

Closed RuiChen-03 closed 6 months ago

RuiChen-03 commented 6 months ago

Add a clear button to the entry widget sorry for closing #36 PR by mistake, so I pull request again. Please review.

quozl commented 6 months ago

Thanks. It looks like this Screenshot_ujj_2024-03-16_10:51:22

The icon is not what Sugar uses for the purpose?

RuiChen-03 commented 6 months ago

Sorry, I didn't fully understand your question. Are you referring to the inconsistency between this button and Sugar's overall style? Or are you suggesting that Sugar already has a button like this?

Regarding the former, since I directly used the set_icon_from_icon_name method from Gtk.Entry, it may appear rudimentary, but I can try to improve it if necessary.Now, it looks like this:

截屏2024-03-16 下午8 18 35

When I click it the content of the text entry will be empty.

As for the latter, when I run the original Sugar, it doesn't have a clear button like this, as shown below:

截屏2024-03-16 下午8 25 12

Is it supposed to have one originally, or is it perhaps a version issue with my Sugar?

quozl commented 6 months ago

Thanks. Please review our icons in the sugar-artwork repository, and see how they are used in other activities. I don't think the icon you have chosen is consistent with the rest of Sugar. Please suggest a different one?

Also, why is the icon inside the shape of the entry widget? The other icons are not.

quozl commented 6 months ago

A potential Sugar icon is the edit-clear. Another one is the edit-undo, which is often paired with edit-redo for stepwise undo and redo.

RuiChen-03 commented 6 months ago

Thanks, I will have a try. For why the icon is inside the shape of the entry widget while the other icons are not, it's because other icons are created by EventIcon and layout by Gtk.Grid; whereas the clear button is created by set_icon_from_icon_name, a method of Gtk.Entry which the entry widget is created by .

quozl commented 6 months ago

Yes, I know why the icon is there from looking at your patch, but you haven't said why you put it there.

chimosky commented 6 months ago

Thanks, I will have a try. For why the icon is inside the shape of the entry widget while the other icons are not, it's because other icons are created by EventIcon and layout by Gtk.Grid; whereas the clear button is created by set_icon_from_icon_name, a method of Gtk.Entry which the entry widget is created by .

Quozl has suggested some icon names you can use above and you can use them with set_icon_from_icon_name which chooses an icon from the current icon theme, and as he'd suggested above, sugar-artwork provides the icons and icon theme used in sugar.

Using the icons he'd suggested would mean you're using one from the icon theme and you might not need to use EventIcon

RuiChen-03 commented 6 months ago

Yes, I know why the icon is there from looking at your patch, but you haven't said why you put it there.

oh, because I think its function is to clear the content of the entry widget, and it seems that most edit clear icon is inside the widget?

RuiChen-03 commented 6 months ago

Thanks, I will have a try. For why the icon is inside the shape of the entry widget while the other icons are not, it's because other icons are created by EventIcon and layout by Gtk.Grid; whereas the clear button is created by set_icon_from_icon_name, a method of Gtk.Entry which the entry widget is created by .

Quozl has suggested some icon names you can use above and you can use them with set_icon_from_icon_name which chooses an icon from the current icon theme, and as he'd suggested above, sugar-artwork provides the icons and icon theme used in sugar.

Using the icons he'd suggested would mean you're using one from the icon theme and you might not need to use EventIcon

Thanks, I have tried to use the edit-clear icon, but I found that the edit-clear.svg in sugar-artwork is transparent or white, so the icon can't be seen in the entry widget although it actually exists.So maybe I need to modify the filling color of the icon.Like change <!ENTITY fill_color "#FFFFFF"> to <!ENTITY fill_color "#000000">.What should I do? Do I need to modify its code to fill it with color before placing it in the icons directory under the chat activity?

quozl commented 6 months ago

Yes, most other edit clear icons are inside the widget, but this is Sugar, we use our own design style, we don't try to make it like everything else. Survey other activities that have similar feature.

I think the problem of the icon not being visible against the entry widget would be solved also by separating them. If I recall correctly, our icons were designed to be against a grey background.

RuiChen-03 commented 6 months ago

Ok.But does it mean I need to use EventIcon instead of theset_icon_from_icon_name method of Gtk.Entry ?Or there is something I don't known about Gtk.Entry ? (Because Chimosky used to suggest to use 'Gtk.Entry', so I used it.) But if necessary, I can also try to use EventIcon.

quozl commented 6 months ago

If you need more detail on how Gtk.Entry works, read the documentation and the GTK source code?

chimosky commented 6 months ago

Ok.But does it mean I need to use EventIcon instead of theset_icon_from_icon_name method of Gtk.Entry ?Or there is something I don't known about Gtk.Entry ? (Because Chimosky used to suggest to use 'Gtk.Entry', so I used it.) But if necessary, I can also try to use EventIcon.

Separating it from the entry widget means that.

RuiChen-03 commented 6 months ago

Thank you.I have tried 3 different looks: this one used EventIcon to separate it, and used edit-cancel in sugar art and I changed the origin filling color to black.

截屏2024-03-22 上午3 26 24

this one used 'EventIcon' to separate it, and used edit-clear in sugar art

截屏2024-03-22 上午3 19 46

this one used Gtk.Entry. Consider both preserving the sugar style and maintainability of the code, maybe this one is not too bad?

截屏2024-03-22 上午2 10 57

Which one is better suited for Sugar?

quozl commented 6 months ago

Which other activities did you look at? It may help to acquire a wider audience such as sugar-devel@ mailing list, or see how other activities do it. See also https://wiki.sugarlabs.org/go/Human_Interface_Guidelines

RuiChen-03 commented 6 months ago

Thanks, I have looked other activities like browser.activity, and the search function of activities which all have similar clear button and I find their clear input button is also in the entry, and occur when the entry sets up, so I do so:

The effect will like the third image above.Please review.

quozl commented 6 months ago

Thanks, I'll test and review.