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.

Sign up for GitHub

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

Jump to bottom

Fix for https://github.com/fyne-io/fyne/issues/5207 #5215

Open
wants to merge 3 commits into
base: release/v2.5.x
Choose a base branch
from
Open

Fix for https://github.com/fyne-io/fyne/issues/5207 #5215

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9856635
Fix for https://github.com/fyne-io/fyne/issues/5207
brucealthompson Oct 19, 2024
8953166
Fixed path manipulation to be for RFC 3986 path
brucealthompson Oct 19, 2024
5bd17a9
Fixes for non "file" repositories"
brucealthompson Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 57 additions & 42 deletions dialog/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,29 +176,36 @@ func (f *fileDialog) makeUI() fyne.CanvasObject {
}
})
f.setView(view)

f.loadFavorites()

f.favoritesList = widget.NewList(
func() int {
return len(f.favorites)
},
func() fyne.CanvasObject {
return container.NewHBox(container.New(&iconPaddingLayout{}, widget.NewIcon(theme.DocumentIcon())), widget.NewLabel("Template Object"))
},
func(id widget.ListItemID, item fyne.CanvasObject) {
item.(*fyne.Container).Objects[0].(*fyne.Container).Objects[0].(*widget.Icon).SetResource(f.favorites[id].locIcon)
item.(*fyne.Container).Objects[1].(*widget.Label).SetText(f.favorites[id].locName)
},
)
f.favoritesList.OnSelected = func(id widget.ListItemID) {
f.setLocation(f.favorites[id].loc)
fileinfo := f.file
createFavorites := true
if fileinfo.startingLocation != nil {
if fileinfo.startingLocation.Scheme() != "file" {
createFavorites = false
}
}

var optionsButton *widget.Button
optionsButton = widget.NewButtonWithIcon("", theme.SettingsIcon(), func() {
f.optionsMenu(fyne.CurrentApp().Driver().AbsolutePositionForObject(optionsButton), optionsButton.Size())
})
if createFavorites {
f.loadFavorites()

f.favoritesList = widget.NewList(
func() int {
return len(f.favorites)
},
func() fyne.CanvasObject {
return container.NewHBox(container.New(&iconPaddingLayout{}, widget.NewIcon(theme.DocumentIcon())), widget.NewLabel("Template Object"))
},
func(id widget.ListItemID, item fyne.CanvasObject) {
item.(*fyne.Container).Objects[0].(*fyne.Container).Objects[0].(*widget.Icon).SetResource(f.favorites[id].locIcon)
item.(*fyne.Container).Objects[1].(*widget.Label).SetText(f.favorites[id].locName)
},
)
f.favoritesList.OnSelected = func(id widget.ListItemID) {
f.setLocation(f.favorites[id].loc)
}
optionsButton = widget.NewButtonWithIcon("", theme.SettingsIcon(), func() {
f.optionsMenu(fyne.CurrentApp().Driver().AbsolutePositionForObject(optionsButton), optionsButton.Size())
})
}

newFolderButton := widget.NewButtonWithIcon("", theme.FolderNewIcon(), func() {
newFolderEntry := widget.NewEntry()
Expand All @@ -224,12 +231,17 @@ func (f *fileDialog) makeUI() fyne.CanvasObject {
f.refreshDir(f.dir)
}, f.file.parent)
})

optionsbuttons := container.NewHBox(
newFolderButton,
f.toggleViewButton,
optionsButton,
)
var optionsbuttons *fyne.Container
if optionsButton != nil {
optionsbuttons = container.NewHBox(
newFolderButton,
f.toggleViewButton,
optionsButton)
} else {
optionsbuttons = container.NewHBox(
newFolderButton,
f.toggleViewButton)
}

header := container.NewBorder(nil, nil, nil, optionsbuttons,
optionsbuttons, widget.NewLabelWithStyle(title, fyne.TextAlignLeading, fyne.TextStyle{Bold: true}),
Expand All @@ -238,16 +250,16 @@ func (f *fileDialog) makeUI() fyne.CanvasObject {
footer := container.NewBorder(nil, nil, nil, buttons,
buttons, container.NewHScroll(f.fileName),
)

body := container.NewHSplit(
f.favoritesList,
container.NewBorder(f.breadcrumbScroll, nil, nil, nil,
f.breadcrumbScroll, f.filesScroll,
),
fileList := container.NewBorder(f.breadcrumbScroll, nil, nil, nil,
f.breadcrumbScroll, f.filesScroll,
)
body.SetOffset(0) // Set the minimum offset so that the favoritesList takes only it's minimal width

return container.NewBorder(header, footer, nil, nil, body)
if f.favoritesList != nil {
body := container.NewHSplit(f.favoritesList, fileList)
body.SetOffset(0) // Set the minimum offset so that the favoritesList takes only it's minimal width
return container.NewBorder(header, footer, nil, nil, body)
} else {
return container.NewBorder(header, footer, nil, nil, fileList)
}
}

func (f *fileDialog) makeOpenButton(label string) *widget.Button {
Expand Down Expand Up @@ -443,13 +455,13 @@ func (f *fileDialog) setLocation(dir fyne.URI) error {
if fav.loc == nil {
continue
}
if fav.loc.Path() == dir.Path() {
if (fav.loc.Path() == dir.Path()) && (f.favoritesList != nil) {
f.favoritesList.Select(i)
isFav = true
break
}
}
if !isFav {
if !isFav && (f.favoritesList != nil) {
f.favoritesList.UnselectAll()
}

Expand All @@ -469,13 +481,16 @@ func (f *fileDialog) setLocation(dir fyne.URI) error {
buildDir = "/"
d = "/"
} else if i > 0 {
buildDir = filepath.Join(buildDir, d)
buildDir = buildDir + "/" + d
} else {
d = buildDir
buildDir = d + string(os.PathSeparator)
buildDir = d + "/"
}
newURL := dir.Scheme() + "://" + buildDir
Copy link
Member

Choose a reason for hiding this comment

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

This assumes that a URI (not URL!) is a scheme followed by a path. This is only true for file URIs. Most contain a host and/or port portion as well.

Copy link
Author

Choose a reason for hiding this comment

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

This assumes that a URI (not URL!) is a scheme followed by a path. This is only true for file URIs. Most contain a host and/or port portion as well.

This variable is actually for a URI, not a URL. I will rename it and check to see if there are other components of the fyne URI with variable name "dir" that need to be included in this constructed URI.

I will create a separate branch for this fix based on your dev branch and resubmit. Give me a little time as I am working on something else right now.

Copy link
Member

Choose a reason for hiding this comment

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

check to see if there are other components of the fyne URI with variable name "dir" that need to be included in this constructed URI.

But why do you pursue the URI re-construction instead of looking into storage.Child? The parsing has all been done already and string manipulation is inherently less safe.

Copy link
Author

@brucealthompson brucealthompson Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I did not write the code you are referring to. I was trying to do the simplest thing possible without rewriting a bunch of existing code.

Not sure why the original author did not choose to use storage.Child(). Was it not evailable when this was originally written?

In any case, I can check into that solution as well.

Copy link
Member

Choose a reason for hiding this comment

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

I did not write the code you are referring to. I was trying to do the simplest thing possible without rewriting a bunch of existing code.

I appreciate that, but sometimes when the existing code contains the bug we need to fix that directly instead of following the old pattern and working around it to minimise the change.

Not sure why the original author did not choose to use storage.Child(). Was it not evailable when this was originally written?

Well, when this only worked with file:// the assumptions held - because it does not (or did not) have any other URI elements like host or port. Those assumptions no longer hold.

Thanks for looking into it.

newDir, err := storage.ParseURI(newURL)
if err != nil {
return err
}

newDir := storage.NewFileURI(buildDir)
isDir, err := storage.CanList(newDir)
if err != nil {
return err
Expand Down
Loading