Skip to content

Conversation

@kubiix
Copy link

@kubiix kubiix commented Sep 23, 2025

This pull request enhances the user interface for managing project pages by adding new functionality and improving the context menu options in the tab bar. The changes focus on enabling page removal, improving icon usage for menu actions, and updating localization strings.

UI improvements and new page management features:

  • Added a "Remove page" option to the page context menu, allowing users to delete pages if permitted (Yafc/Widgets/MainScreenTabBar.cs, Yafc/Data/locale/en/yafc.cfg). [1] [2]
  • Updated context menu buttons to consistently use relevant icons for actions such as edit, open as secondary, close secondary, duplicate, and remove page (Yafc/Widgets/MainScreenTabBar.cs).

Icon and localization updates:

  • Introduced the new Secondary icon in the Icon enum to visually represent secondary page actions (Yafc.UI/Core/Structs.cs).
  • Added the remove-page string to the English locale file for proper localization of the new menu option (Yafc/Data/locale/en/yafc.cfg).
{EB2D77C8-B12E-4B73-98B1-2AF28639DDAA}

@kubiix kubiix requested a review from shpaass as a code owner September 23, 2025 21:26
Copy link
Collaborator

@veger veger left a comment

Choose a reason for hiding this comment

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

Thanks for this nice improvement!

As mentioned in the other PRs, a changelog entry would be appreciated

@kubiix
Copy link
Author

kubiix commented Sep 23, 2025

Changelog filled

MainScreen.Instance.SetActivePage(copy);
}
}
if (gui.BuildContextMenuButton(LSs.RemovePage, icon:Icon.Delete, disabled: !page.canDelete)) {
Copy link
Collaborator

@DaleStan DaleStan Sep 24, 2025

Choose a reason for hiding this comment

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

Suggested change
if (gui.BuildContextMenuButton(LSs.RemovePage, icon:Icon.Delete, disabled: !page.canDelete)) {
if (gui.BuildContextMenuButton(LSs.DeletePage, icon: Icon.Delete, disabled: !page.canDelete)) {

For consistency with ProjectPageSettingsPanel, this should probably use the same string.

open-secondary-page=Open as secondary
shortcut-ctrl-click=Ctrl+Click
close-secondary-page=Close secondary
remove-page=Remove page
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even if we do want separate strings (Factorio has four "Cancel"s and three "Back"s in English), they should probably have the same content, to avoid questions about the differences between 'remove' and 'delete'.

Copy link
Owner

Choose a reason for hiding this comment

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

Agreed. We can reuse the delete-page instead.

bool isSecondary = screen.secondaryPage == page;
bool isActive = screen.activePage == page;
if (gui.BuildContextMenuButton(LSs.EditPageProperties)) {
if (gui.BuildContextMenuButton(LSs.EditPageProperties, icon:Icon.Edit)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (gui.BuildContextMenuButton(LSs.EditPageProperties, icon:Icon.Edit)) {
if (gui.BuildContextMenuButton(LSs.EditPageProperties, icon: Icon.Edit)) {

Our code style calls for spaces here, and missing spaces will cause problems for those of us who are using the recommended pre-push hook.

Version:
Date:
Features:
- Add icons to Page DropDown items
Copy link
Owner

Choose a reason for hiding this comment

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

Might want to point out that it's about the right-mouse-button menu, just to be clear.

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.

4 participants