Skip to content
This repository was archived by the owner on Jul 19, 2022. It is now read-only.

Commit 494fc12

Browse files
hojberghojberg
committed
Ensure Definitions are opened via Route changes
We want the URL to be main way definitions are opened and fetched. Previously this was an afterthought such that we fetched a definition and then changed the URL, resulting in broken back-button behavior. This changes how we load definitions such that a URL is changed which, in a route handler, causes the Workspace to open and fetch that definition, with 1 exception: when a definition is opened from another definition, it is important that we open the new definition relative to the originating definition (above it or below it). In this case we open the definition, then change the URL, which subsequently tries to open the definition again; seeing that the definition has been opened already the fetch attempt is cancelled.
1 parent 105a4b9 commit 494fc12

File tree

3 files changed

+57
-47
lines changed

3 files changed

+57
-47
lines changed

‎src/App.elm

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,20 @@ update msg ({ env } as model) =
134134
( model, Cmd.none )
135135

136136
UrlChanged url ->
137-
-- URL changes happen when setting focus on a definitions.
138-
-- Currently, the URL change is a result of that as oppose to focus
139-
-- being a result of a URL change
140-
( { model | route = Route.fromUrl env.basePath url }, Cmd.none )
137+
let
138+
route =
139+
Route.fromUrl env.basePath url
140+
in
141+
case route of
142+
Route.Definition _ ref ->
143+
let
144+
( workspace, cmd ) =
145+
Workspace.open env model.workspace ref
146+
in
147+
( { model | route = route, workspace = workspace }, Cmd.map WorkspaceMsg cmd )
148+
149+
_ ->
150+
( { model | route = route }, Cmd.none )
141151

142152
ChangePerspective perspective ->
143153
replacePerspective model perspective
@@ -165,7 +175,7 @@ update msg ({ env } as model) =
165175
keydown model event
166176

167177
OpenDefinition ref ->
168-
openDefinition model ref
178+
navigateToDefinition model ref
169179

170180
ShowModal modal ->
171181
( { model | modal = modal }, Cmd.none )
@@ -200,7 +210,7 @@ update msg ({ env } as model) =
200210
in
201211
case outMsg of
202212
PerspectiveLanding.OpenDefinition ref ->
203-
openDefinition model2 ref
213+
navigateToDefinition model2 ref
204214

205215
PerspectiveLanding.ShowFinderRequest ->
206216
showFinder model2 Nothing
@@ -227,7 +237,7 @@ update msg ({ env } as model) =
227237
model4 =
228238
{ model2 | sidebarToggled = False }
229239
in
230-
openDefinition model4 ref
240+
navigateToDefinition model4 ref
231241

232242
CodebaseTree.ChangePerspectiveToNamespace fqn ->
233243
fqn
@@ -251,7 +261,7 @@ update msg ({ env } as model) =
251261
( { model | modal = NoModal }, Cmd.none )
252262

253263
Finder.OpenDefinition ref ->
254-
openDefinition { model | modal = NoModal } ref
264+
navigateToDefinition { model | modal = NoModal } ref
255265

256266
_ ->
257267
( model, Cmd.none )
@@ -268,19 +278,9 @@ update msg ({ env } as model) =
268278
-- UPDATE HELPERS
269279

270280

271-
openDefinition : Model -> Reference -> ( Model, Cmd Msg )
272-
openDefinition model ref =
273-
let
274-
( workspace, wCmd, outMsg ) =
275-
Workspace.open model.env model.workspace ref
276-
277-
model2 =
278-
{ model | workspace = workspace }
279-
280-
( model3, cmd ) =
281-
handleWorkspaceOutMsg model2 outMsg
282-
in
283-
( model3, Cmd.batch [ cmd, Cmd.map WorkspaceMsg wCmd ] )
281+
navigateToDefinition : Model -> Reference -> ( Model, Cmd Msg )
282+
navigateToDefinition model ref =
283+
( model, Route.navigateToDefinition model.navKey model.route ref )
284284

285285

286286
replacePerspective : Model -> Perspective -> ( Model, Cmd Msg )
@@ -333,7 +333,11 @@ handleWorkspaceOutMsg model out =
333333
showFinder model withinNamespace
334334

335335
Workspace.Focused ref ->
336-
( model, Route.navigateToByReference model.navKey model.route ref )
336+
let
337+
x =
338+
Debug.log "ref focus" ref
339+
in
340+
( model, Route.navigateToDefinition model.navKey model.route ref )
337341

338342
Workspace.Emptied ->
339343
( model, Route.navigateToCurrentPerspective model.navKey model.route )

‎src/Route.elm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ module Route exposing
22
( Route(..)
33
, fromUrl
44
, navigate
5-
, navigateToByReference
65
, navigateToCurrentPerspective
6+
, navigateToDefinition
77
, navigateToLatestCodebase
88
, navigateToPerspective
99
, perspectiveParams
@@ -263,8 +263,8 @@ navigateToLatestCodebase navKey =
263263
navigateToPerspective navKey (ByCodebase Relative)
264264

265265

266-
navigateToByReference : Nav.Key -> Route -> Reference -> Cmd msg
267-
navigateToByReference navKey currentRoute reference =
266+
navigateToDefinition : Nav.Key -> Route -> Reference -> Cmd msg
267+
navigateToDefinition navKey currentRoute reference =
268268
navigate navKey (toDefinition currentRoute reference)
269269

270270

‎src/Workspace.elm

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ init env mRef =
5555

5656
Just ref ->
5757
let
58-
( m, c, _ ) =
58+
( m, c ) =
5959
open env model ref
6060
in
6161
( m, c )
@@ -116,7 +116,7 @@ update env msg ({ workspaceItems } as model) =
116116
in
117117
( { model | workspaceItems = nextWorkspaceItems }
118118
, cmd
119-
, openDefinitionsFocusToOutMsg nextWorkspaceItems
119+
, None
120120
)
121121

122122
IsDocCropped ref res ->
@@ -157,7 +157,7 @@ update env msg ({ workspaceItems } as model) =
157157
WorkspaceItemMsg wiMsg ->
158158
case wiMsg of
159159
WorkspaceItem.OpenReference relativeToRef ref ->
160-
openItem env model (Just relativeToRef) ref
160+
openReference env model relativeToRef ref
161161

162162
WorkspaceItem.Close ref ->
163163
let
@@ -218,11 +218,6 @@ type alias WithWorkspaceItems m =
218218
{ m | workspaceItems : WorkspaceItems }
219219

220220

221-
open : Env -> WithWorkspaceItems m -> Reference -> ( WithWorkspaceItems m, Cmd Msg, OutMsg )
222-
open env model ref =
223-
openItem env model Nothing ref
224-
225-
226221
replaceWorkspaceItemReferencesWithHashOnly : Model -> Model
227222
replaceWorkspaceItemReferencesWithHashOnly model =
228223
let
@@ -232,7 +227,29 @@ replaceWorkspaceItemReferencesWithHashOnly model =
232227
{ model | workspaceItems = workspaceItems }
233228

234229

235-
openItem : Env -> WithWorkspaceItems m -> Maybe Reference -> Reference -> ( WithWorkspaceItems m, Cmd Msg, OutMsg )
230+
open : Env -> WithWorkspaceItems m -> Reference -> ( WithWorkspaceItems m, Cmd Msg )
231+
open env model ref =
232+
openItem env model Nothing ref
233+
234+
235+
{-| openReference opens a definition relative to another definition. This is
236+
done within Workspace, as opposed to from the outside via a URL change. This
237+
function returns a Focused command for the newly opened reference and as such
238+
changes the URL.
239+
-}
240+
openReference : Env -> WithWorkspaceItems m -> Reference -> Reference -> ( WithWorkspaceItems m, Cmd Msg, OutMsg )
241+
openReference env model relativeToRef ref =
242+
let
243+
( newModel, cmd ) =
244+
openItem env model (Just relativeToRef) ref
245+
246+
out =
247+
openDefinitionsFocusToOutMsg newModel.workspaceItems
248+
in
249+
( newModel, cmd, out )
250+
251+
252+
openItem : Env -> WithWorkspaceItems m -> Maybe Reference -> Reference -> ( WithWorkspaceItems m, Cmd Msg )
236253
openItem env ({ workspaceItems } as model) relativeToRef ref =
237254
-- We don't want to refetch or replace any already open definitions, but we
238255
-- do want to focus and scroll to it
@@ -243,7 +260,6 @@ openItem env ({ workspaceItems } as model) relativeToRef ref =
243260
in
244261
( { model | workspaceItems = nextWorkspaceItems }
245262
, scrollToDefinition ref
246-
, openDefinitionsFocusToOutMsg nextWorkspaceItems
247263
)
248264

249265
else
@@ -261,24 +277,14 @@ openItem env ({ workspaceItems } as model) relativeToRef ref =
261277
in
262278
( { model | workspaceItems = nextWorkspaceItems }
263279
, Cmd.batch [ Api.perform env.apiBasePath (fetchDefinition env.perspective ref), scrollToDefinition ref ]
264-
, openDefinitionsFocusToOutMsg nextWorkspaceItems
265280
)
266281

267282

268283
openDefinitionsFocusToOutMsg : WorkspaceItems -> OutMsg
269284
openDefinitionsFocusToOutMsg openDefs =
270-
let
271-
toFocusedOut workspaceItem =
272-
case workspaceItem of
273-
Success ref _ ->
274-
Focused ref
275-
276-
_ ->
277-
None
278-
in
279285
openDefs
280-
|> WorkspaceItems.focus
281-
|> Maybe.map toFocusedOut
286+
|> WorkspaceItems.focusedReference
287+
|> Maybe.map Focused
282288
|> Maybe.withDefault Emptied
283289

284290

0 commit comments

Comments
 (0)