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

fixes #16315 dependency browser layout feels wrong #16343

Conversation

n1toxyz
Copy link
Contributor

@n1toxyz n1toxyz commented Mar 24, 2024

This PR proposes a fix for issue #16315. It changes the UI to provide the widgets with their respective space. For this some of the buttons are moved into an SpActionBarPresenter.

See the screenshot below for the new look.
16315-new-ui

This change removes the text from the refresh button, making it an icon only button.
Additionally the icon is changed from #refreshIcon to #glamorousRefresh to make its function more clear to the user.
Lastly the expansion of the button was disabled in DADependencyTreePresenter class>>defaultLayout.
…ctions.

This change adds a SpActionBar Presenter to DADependencyTreePresenters and changes its layout to use it to display the action buttons.
The action buttons were changed to accomodate their usage in the action bar, which meant removing their text and changing some of 
their icons for clarity.
@Ducasse Ducasse closed this Mar 24, 2024
@Ducasse Ducasse reopened this Mar 24, 2024
@@ -4,6 +4,7 @@ A PDPackageAnalyzerTreeModel shows all dependent packages from a set of packages
Class {
#name : 'DADependencyTreePresenter',
#superclass : 'DAPackageTreePresenter',
#classTraits : '{} + TraitedClass',
Copy link
Member

Choose a reason for hiding this comment

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

I think there is a problem with the Tonel export here.

I've seen messages about this but I did not follow what causes this and what should be done when this happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see... I didn't notice it. Iceberg didn't show it either. I was using a fresh image when working on the PR. Is there something I can do to fix this?

@Ducasse
Copy link
Member

Ducasse commented Mar 25, 2024

Argh again this bug!
Sorry for the inconvenience.
We often edit the file by end. We are looking for a way to reproduce it.

@n1toxyz
Copy link
Contributor Author

n1toxyz commented Mar 25, 2024

Is it possible for me to edit it by hand or do I need to do a new PR? Do I just have to delete the extra line?

@Ducasse
Copy link
Member

Ducasse commented Mar 26, 2024

I imagine just remove the extra line and the preceeding ,

@jecisc
Copy link
Member

jecisc commented Mar 26, 2024

I edited the file

@jecisc jecisc merged commit 6b848c6 into pharo-project:Pharo12 Mar 26, 2024
1 of 2 checks passed
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.

None yet

3 participants