GitLab Frontend group, 9 Sep 2020

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Frontend Foundations – Pairing on Keyboard Shortcuts Modal

Description

MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41800

A

So yesterday I erased two, mrs regarding jquery, um so the jquery- um let me just maybe share you- know at least part of my screen, I'm asked by me, so I was really positive positively astonished.

A

um So I created this, mr um that just moves the two of the jquery plugins, like the add who and scroll to to the pages where it's used right, okay, um so same idea, I think we did it earlier with the jquery bait for images right.

A

One actually won't do anything, because it's used in the hi david, it's used in the gitlab flavored markdown, so it's still loaded in main.js, but actually jquery. Scroll too will do something and just be loaded on those pages where it's used tests are passing and, funnily enough, um we don't use it in that many pages we just seem to be using it in, like I don't know uh how many files did I touch with this one.

A

uh We just use it in five places so like uh the effort to replace it will be like really low right um yeah.

A

um But while I was doing that, I found like these horrendous because I thought hey once we move those we probably I wanted to check because we have the window.jquery and window.dollar assignment right. I was like hey. Maybe we can remove it, but then I stumbled about these awful things.

A

Yes, so, like oh see, it's evil, it has 666 bytes um like like some like javascript that is rendered based on erb files, and so this is, for example, for saving the um the preferences, like the user preferences. If you change like the themes and stuff um and.

B

Yeah, there's a bunch of these um and they're they're they're, not linted, because they're, not javascript files and they're not uh compiled in any sense. um You can't use babel stuff or you know, uh es6 syntax. I guess you can now because we've dropped most of the browsers that don't support it, but like it's just bad, it's just a bad idea to have this.

A

Yeah, it's it's the same thing, because the second amount that I'm raised yesterday was uh removing flash on the window object right. Similar idea, like we have this deprecated flash thing yeah and we even had sometimes we had like still had like global flash, and I just searched for those was like hey.

A

We just have two global flash, so let's remove it and the linter will find everything and then obviously the pipeline breaks if you remove it from window flash exactly because of this right, yeah so yeah and, to be honest, like I, don't have a good solution for that stuff. um The problem.

B

Is too it's hard to hardly even find it all? I guess we could, because there are some of these, where it's like, strictly like dot js.erb, but there's also, I mean there's nothing stopping anyone from putting like a script tag in the middle of any random haml file, uh and I think we do that in a couple places.

B

A

For example, this one here right, like oh wow, I love these commits like um update.js.hemo.

B

Yeah, seven years ago so hard to yeah, we didn't really have a robust uh front-end team seven years ago. So it's hard to blame uh dimitri.

A

No, I I don't wanna blame dimitri, it's just like. I also don't understand how these update js. These are just conditionally rendered, based on on on this right best, based on the remote true right.

A

This is like I, I just wonder whether it makes sense to go through all of these and remove jquery from it, because then we can remove our global jquery because, to be honest, like hey, this has been working for you know, seven years.

B

A

B

The the better ultimate solution is to remove this embedded javascript and put it in a page specific javascript file. I mean I don't know if we're gonna take the effort to refactor these, how much more effort is it to just move it.

C

D

I uh I had an issue about that. Well, I didn't create this issue. Apparently this is a two-year oldish issue uh from one of our former team members and I.

A

Just wanted to say: is it by disabled account.

D

No, no, there isn't actually a.

B

D

Behind the name.

B

I know bryce opened a whole bunch of issues way back in the day.

D

And no this one's like conor mcgregor. Do you remember kenner.

A

Yeah, oh yeah yeah. I I I see connor from time to. I never had the pleasure of conor because I joined after he left the company and I was always is always like. I see his name popping up either in commit messages or like in the licenses dependency licenses stuff, because apparently he approved a bunch of them back in the day. And it's like you look through it. It's like. I approve the mit license right, like yeah yeah, so.

D

Apparently, this is of interest to many a product managers across gitlab because of the potential risk of actually missing some escape javascript or something so they kind of want to prioritize this and have engineers of each team move these files to its corresponding bundle.

D

A

Wow, that's uh it's really great. um I mean okay, how can we support this? Does it make sense to create issues for him.

D

Let me post the issue on the document, so you can take a look at it.

B

Let me just find the document actually just update this two-year-old issue add a little like checklist of each of the instances, there's only about a dozen right, at least the ones that we.

A

Found wait. Let me let me quickly, let me quickly close all the stuff that I don't want to have on the recording, like my calendar and so on, and then I can share all my emails or what else maybe do I have? No private slack is always a bad one.

A

Calculator might reveal like I don't know what a calculator might reveal. um Okay, new share. Let me share all of my screen. This should be fine. You know uh gitlab.

A

Find and I just searched for js hammer.

A

A

Yeah, it's not it's not that many right like how many are these one, two, three, four, five, six too many to count. 17.

B

um Should we also check for just script tags within hamil? uh There might be some of those as well.

A

Is it like these? These are the the plane thingy right, like the what is happening. Why is my computer.

A

Beach ball of death.

A

A

Okay, this is interesting.

A

uh um Mine dot app like.

C

C

B

A lot of the script tags, I'm looking for I'm looking at, are basically just ways to inject raw json data to pass to the front end from the back end and, I suppose that's an acceptable use. I just want to make sure that there isn't any of this. This, like you, know using jquery to do something.

A

I mean one of the things that one can probably use is like uh we could search for uh like actual dollars in hammer files, yes, um and then at least we find like some I uh this. These are all the ah what source branch, source branch html. So wait is this creating like html save stuff that is then used in a view template.

E

D

There are a few instances of uh using x template uh x-template on a couple of hammer views like the uh what's. It called the issue boards. The issue boards use the x-template to declare.

B

Yeah they use the runtime view, template compiler and we- yes, that's yeah. That's been another long standing issue. We have to take care of some time.

B

um Also it will reduce the size of our view, library that we load by about half. I think so. That would be nice to get rid of. Also, I'm not even sure if the next version of view includes the runtime compiler.

B

Might affect our ability to upgrade.

D

I updated the list below because there's a bunch of files that do not apply anymore. There's a comment down right below.

A

But these are like html harm. What's about the js stuff.

B

Well, those ones are.

A

Wait these ones are evolved by javascript.

D

Some of them where some of them were updated not to do emails anymore, but there is some risk of leaving that only them like that.

A

So like this is rendering stuff and oh gosh, this is like how. How does one even go about these like.

B

The uh the debugging code, that's generated by other code, is just the worst. That's why the the the issue boards is such a quagmire. I don't think anyone's wanted to tackle it yet.

A

I think issue boards are on a good progress. If I recall correctly, we migrated a few for the karma to jest efforts and we just migrated a few to a view yeah and but these ones like. What can we do about it? Like.

A

Like this is just to show some modal right.

D

A

Render shortcuts like what shortcuts html.

A

Wait, this is for the normal shortcut question mark thingy.

D

Yes, yes, that's the one.

A

Oh wow, it's actually broken.

C

B

What arrow up just isn't, rendering correctly.

D

The thing is that we remove phone awesome so remember that there is no.

A

Yes, it's funny, but but there is one there's actually one. This should then already be a gitlab ui icon. You should but.

D

A

Because merge requests, I noticed because one of my two team members.

A

Merged moment wording what he is his username, a different one. ah It's a signed. uh I wondered why he doesn't have that many uh offer username.

B

I'm gonna say it looked like he had only authored six merge requests in total, which was doesn't seem right.

A

But it's it's replaced here so and the code says: did he replace it in the keyboard yeah? It should be sprite. Icons like it's weird is: is it maybe like a canary non-canary thingy? Yes,.

D

A

Canary, but what why am I getting stuff? That's canary and stuff? That's not canary!

A

ah Maybe the keyboard shortcuts are not just in the scope of canary anyway, um doesn't matter too much, um yeah, okay, but in this case, for example, this one would pretty easy would be pretty easy to move, because we just rendered the shortcuts from the html or from ahamel and then.

A

This even makes no sense at all like.

D

A

D

We we have the template on on hamel and then the template is rendering well, it's called by javascript. So we will have to move this model to view.

B

I just don't understand why we're even like, so it's it's rendering this server side and and embedding it as a string which then gets appended to the body like why not just yes render it.

C

B

The body directly correctly.

C

D

Yeah, I'm not sure either is, is.

A

It doing is it doing that you know via an ajax call. So if I'm no.

D

This is all back in well. Server side render.

A

Yeah but the question is like what happens if I hit question mark, does it do anything like if I hit quit.

A

See, oh okay, it just happens when I, when I hit question mark it sends a request. Oh.

B

So this is okay.

F

A

Where's the response response, payload.

C

A

A

This is basically deferred rendering right.

A

Awful deferred, rendering.

A

But yeah I mean especially in this case. It should be pretty easy, because we would just need to move this file into a view, template which has nothing server rendered, which should be pretty easy right because uh we have translations.

A

Oh browser platform mac, so this is an ugly one, but should be should be doable somehow like. Oh, I think we already saved that in the dom right, whether you're on a mac or not like don't. We have like something like window gone uh window, gone platform or something like that window.

B

We have other places where yeah we show like the the command symbol instead of option or whatever it is in windows.

A

I definitely saw our code, the other data. Does this like script? Nonce window gl is mac. Yes, so it's window.gl is mac. No. What.

A

Wait a second ah window client.

C

A

Yeah, I think this could be movable, and but now it would turn into a pairing session. If we just do that right.

A

E

A

This is so weird like: how do we call it? Actually I mean: how do we do that request.

A

Like what triggers this.

B

A

See if ruby mine is happy shortcut, no, it's shortcut.js right.

B

Under behaviors we have init page shortcuts, maybe there's something there.

A

Yeah and there's a merge request that literally touches that file by tim uh short cuts code, splitting shortcut.js for reduction.

A

Import from shortcut shortcuts so.

B

Oh wait: this is actually initializing the shortcuts themselves, not the modal. That tells you about them.

C

A

But the question mark on toggle help.

A

Right because the.

C

A

Shortcut this enabled help this enabled help what shortcuts modal trigger.

A

Click more help.

A

ah Yes, so apparently we have a keyboard shortcuts, help.

B

Yeah so it's saved to gone dot, shortcuts, underscore path and there's just an axios request to grab that.

C

D

Oh wow and then we then we do a global e-ball of that yeah.

A

Yeah, you know what you know. What folks I don't know. uh This is a pretty interesting I'm. I think we could do the shortcuts one unless someone else wants to talk about something um we could just do it. What do you think should we do it and.

D

A

The vote a little bit better.

D

Yeah, let's, let's do this one.

E

A

uh New file short cuts, help dot view.

A

B

Sure I don't wanna like shed this while we're here, because we just wanna fix this one problem but like couldn't there be a way to have the front end since we're doing all this initialization of the actual shortcuts have a way for it to just generate the shortcuts help menu from.

D

All of that at the same time, and then we have a single source of truth for our shortcuts.

D

I mean we use mouse trap and the thing with metal strap is that mousetrap binds uh whatever like it gets called once globally, and then you can start buying multiple shortcuts later wow. So I remember doing that for the metrics dashboard, where I actually use mousetrap to add a couple of shortcuts to the metrics dashboard and when I added the extra ones on to the shortcuts menu, they don't show up like uh you, you have to add those manually to the hd.

B

Yeah, well, that's what I'm saying is like. Maybe we create a way where, like some wrapper method, where like when you bind uh a keyboard item like a keyboard shortcut, it will both uh actually do. The the binding with mousetrap it'll also store that in some array of like here's, our list of bound shortcuts and then, when you hit the question mark that will be just generated from those.

D

I mean that that that sounds doable, but.

B

Like I said, this is out of the scope of what we're doing right now. I just thought like yeah. It was improvement.

D

We'll have to grab for every single shortcut, yeah.

B

Well, it looks like they're all here under behaviors, slash shortcuts, but maybe there's some page specific ones that are just not captured in this list.

D

The web ide also has uh some mousetrap bandings too.

A

So how do we convert this best uh modal header.

B

So this this axios thing, I suppose um right now it it uses don dot shortcuts path. Let's have that point to just a hammel template. That's um that's! The rendered uh shortcuts stuff, like the the html itself, rather than a javascript file.

A

Why why don't we? Just because it's a static file, move it like to view or something.

B

We could do that too. Yeah.

A

uh This is our normal translation, thingy right.

E

A

Close, so this is times right, okay, times area, hidden.

A

Okay, so I assume that you know modal header, we pre probably can even use the gitlab ui model right.

A

Oh, by the way, this is like horrible horrible.

A

Javascript right, like really horrible.

A

Do we translation simply like this right, like sorry, if I haven't been writing view for a longer period, yeah like this okay.

F

Yeah that should work.

A

Yeah, this is annoying. Is that isn't there one more efficient way to do this, like.

B

um I mean you could try and cobble together a regular expression to do a search and replace and make this faster. But.

A

I don't know it's like copy path.

E

E

A

uh Copy, let's just overwrite this file.

A

I mean ah regular expression. I mean you're on the right way right like.

B

I don't I'm not super familiar with ruby mine, but in vs code I can just do a find and replace and use regular expressions to target like these, these td tags. It looks like.

A

Yeah yeah exactly it, I think at least for the td tags. You know it looks reasonable to do uh you know, basically everything that happens in the body like okay, let's copy everything that happens in here.

A

Modal body, oh gosh, now why does it break the thing.

B

I would have started with the the copy you had. That was just the rendered copy and just done regular expressions.

A

On that I mean okay.

A

A

uh Oh, it's funny. ah Okay, these are the headlines right yeah, so I mean the rendered stuff looks good right, so we can just use the oh wait, I'm by the way, I'm on the wrong branch. Aren't I no! It's just not updated.

A

um And we can replace this by td td and then it will be hitting the right keys this, and we obviously need also the curly brackets right.

B

Oh, you just replaced all the th's with tds.

A

Oh yes, you're right.

C

A

Okay shift: l shift, I okay, so this is interesting because essentially we need to replace the the uh okay okay. I have an idea.

B

Let's just create a um what you call it uh wow drawing a blank view. The.

B

Computed property, uh that's just like platform, specific yeah roll character or whatever.

A

Yeah, that's that's a good idea.

A

I mean doesn't even have to be computed. Right uh could just be like. um Oh that's super interesting because it's also like um return. What was it like window? Gl client is mac. Yeah.

A

So the problem is that now is it's html right, so let's maybe find that character, because you know we are in utf-8 mode. So should be fine right to just do this, and then we can replace this with that.

A

C

A

Oof now it it wants all of those contents to be marked for translation. Like n b sounds like a good use case for our uh improving our linda. That kbd should not be translated right.

B

For now we could do an eslint disable or something.

A

Okay, so this will be shift.

A

A

Okay, how many of those do we have? Oh.

E

A

Why is this like? Oh yeah, okay, this looks better okay, uh so it's just the same. So it's just like gl icon, and then we have this okay.

A

Whatever okay gl icon and then it's name right and then equals and just like that.

A

Okay, obviously self-closing, replace, replace, replace.

E

E

A

Okay, replace all gl icon shift. um Maybe we can.

A

Oh wow, I don't think we want to translate them right.

A

We probably what's the rules name.

E

Oh, what what is.

A

Happening: okay, nothing is happening.

B

I don't know because I don't use international keyboards, but do they translate things like shift and enter, or is it kind of just a thing.

A

Yeah in in german, you would, for example, say um child, which is like switcher for shift um yeah, but everyone should understand shift, and maybe we can then translate it in one. Go. I don't, but we definitely don't translate. J.

D

I mean the only one, at least from my spanish keyboard is the caps lock? uh That's the only one that is translated that and and delete the other ones remain. The.

C

C

A

Yep, so how is this rule called.

A

Oh so we actually have a bug in our implementation. This is good to know, but I was interested in this rule because then we can just be like.

A

Eslint disable and this right and then we should be happy about that rule. We find this is our problem, so let's maybe backslash it and be fine right.

A

And merge tags. Okay, so this looks good to me like this should work now the question is like: how would we show it right if it's a gitlab model like in this in this shortcuts function, that does the shortcuts path right.

B

Yeah well, uh we could just do a dynamic import. Yes,.

A

Then we so you're saying we just do return import shortcuts, help right then default shortcuts help. Essentially, oh this needs more brackets right and then we just do a return new view and we maybe const you.

B

Can actually handle all the promise evaluation stuff, that's on its own. If we just register the components and like we normally would and uh pass it a function that returns a promise, so you can just pass it and import uh dynamic, import.

A

ah Yeah yeah you're right, so we you, you would basically be like return new view. Yeah uh document create element, const l and we just do l, uh oh god, how? How now I'm this is. How do we do this components would be.

B

I guess: is there a mounting point that we should that exists in? I don't.

B

Know did so we're just going to create an element: that's like prepended to body or postpended. How do we do this with modals generally, that are dynamically.

B

A

B

Probably good enough to just do that.

A

uh Shortcuts help so you're saying we're doing something like this uh yep.

A

Shortcut help dot view.

A

Okay, I'm just commenting this out because whatever right and then we just do a render function.

B

A

The question is like: how do you reference this in a render function if it's imported that way.

B

uh Just shortcuts help same way that you've finally registered.

A

Like this, but it's now undefined right, a string.

C

B

It could actually just be like the the kebab case that we use everywhere else. That's actually rendering it in the template.

C

B

Cut it'll it'll work either way, but maybe this okay, I'm not sure.

A

Okay, so we just do this right uh new view element. Is it written like this or is it like element?

A

D

A

Okay, so we comment this out and then I am not the smartest person sometimes so I could have gdk started like ages ago. In the background, right and what's really interesting, though, is like in a toggle button because look what init toggle button actually does. So we have a view app within a view app, um which is the toggle button, that's at the top here. So at least here we can then use a real, real import right.

A

ah So, let's maybe just do that import.

A

Shortcuts toggle from.

A

Shortcuts toggle dot view.

A

Then we have this as components shortcuts toggle, and here we would just use shortcuts toggle and that's it right.

A

Then we don't need in a toggle button. We don't need the nonsense, huh sorry, um axials and shortcuts. So what what else did we do.

C

A

Location length what's location like this, this, this piece is a little awkward.

B

Yeah, what is going on there.

A

Toggle help location.

A

I have a feeling- and this is just like location, so wait- location length, results, results, push.

E

Reach location.

B

So this yeah it it will return results from that then, and then it'll pass it into init toggle button. So a knit toggle button might take results of this location array in as a primary.

A

Maybe it would have been good to have the gdk running before we do something like this.

A

Close our tabs.

A

There was probably nothing important open.

D

Famous last words.

A

Yeah famous last words.

B

Yeah, I don't know if anything is actually done with that location, so it compiles this results array using location, but it does nothing with it.

A

Yeah, it does look, results, push location e show, so I assume that some of these might be hidden.

B

Oh in this show.

A

Yeah, so so it might be that we have like certain, like the wiki stuff or whatever that is just shown conditionally or was shown conditionally shortcut. Mappings.

D

uh Perhaps the council lock will be in order just to figure out which ones are shown which ones are not.

C

A

Oh request ran longer, then that's always good.

E

A

Let me oh location is defined but never used, oh yeah. That makes sense so you're saying we should do this and not do that right.

A

If modal length, okay.

A

A

ah User sign in this is a good sign. At least the redirect is correct.

A

And compiling is successful.

A

Do I have anything else of me? Maybe I should close. The other browser often helps.

A

ah Yes, I actually caused this bug.

A

It's also a fun one is like back when we had the cee split. We had green fluffy cons for ee and blue favicons for ce, uh but I assume nev. No one is ever seeing the blue favicon in development, so we can now probably just get rid of that distinction anyway. So question mark. Does this work.

C

A

Axios is not defined, yes, because I removed it.

A

Let me unremove.

B

Me max here is used somewhere else in this file.

A

Nope, uh let's maybe just discard these changes for now, and also the inner toggle button ones.

C

A

Shortcut helps for you, okay, recompiling, okay, question mark axios is not defined. It's now there here, axios.

A

You're typing in the console question mark.

A

You know, maybe we should sell the front end foundation hours as um as pair programming sessions and then more people would join um yeah okay. So ah we wanted to do some console logging right.

C

A

So we we wanna console log results.

A

Console log location- maybe I think the data is good data.

A

A

And results: do we need to lock anything else.

B

A

So recompiling.

E

A

Wow, okay, this seems to be so. The location is empty.

A

It's the javascript, essentially, location, yeah location is empty, so obviously results. Nothing happens. Right, um toggle, help.

D

What happens if you go to the web, id will location will be populated.

A

Oh, this is a good question. uh How do I get to the web ide the fastest, probably like here and clicking open web id? I also am confused because we have this other shortcuts stuff like here in the behaviors like, for example, merge projects. Issues right project issue is new, so projects issues new, so new project, web id you're saying and I'm saying, new projects, yeah project name, question: mark okay. This looks exactly the same to me.

A

So doesn't do anything to the help.

E

A

And this does just the same thing right wait. Is this a different help than this one like looks.

C

The same to me.

C

A

C

A

Dark mode colors are broken, but not our concern right.

A

A

I don't know what this location thing does. Is it tested somewhere.

A

Or should we just go back in history, I really don't think.

B

It does anything I'm looking at this and well.

A

So, let's what was the shortcut again t right? Okay,.

B

So it's looking for a class that is the same as location.

A

B

See any classes in this, but.

A

When was this introduced like I assume that this stuff is like huh okay, I give you the benefit of the doubt mike. uh It seems like you run prettier on it.

A

Convert shortcuts to use axios I give paul phil also been, ah did not exist. Oh that's. Why? um ah So? It probably goes back to es65 all the things so, which means we'll end up finding.

B

A copy script file with the source of this.

F

A

Oh- and this is like.

A

All right, let's go to the parent, commit.

A

Okay, so view file at now, we should find some shortcuts short cuts, dot, js coffee right, oh wow. How is the search so performant even.

B

Wow, it's it's just a really ancient commit yeah.

A

Yeah for a really ancient commit like it's. It's super okay, right, okay, location, toggle, help model.

B

Alfredo, I haven't seen him in a long time right.

B

A

Location awesome shortcuts, oh robot, he's still around location, l show in location like awesome shortcuts. I assume that these awesome shortcuts are like okay, mousetrap rails.

A

When shortcuts and so on,.

A

Okay, do we have anything with location.

A

Show help location window, location.

B

Was passed to show help before I don't think the later incarnations do that.

A

I mean there is the show help. My assumption would be that the show help doesn't work in all places or wasn't back then wasn't working in all places right that, like hey, no 4l in location like this, makes no.

E

E

A

Show help show help ah shortcut, even here, it's not passing anything to show help at enabled help. So this is the location right. What's at enabled help.

A

Hidden shortcut projects so do we still have these things somewhere.

A

A

Yes see here, they have classes here like hidden shortcut and so on. I think this doesn't do anything anymore like because here they have a hidden shortcut and if you are like on a page, if you're on a merge request or if you're on the issuable you're loading, the special stuff and then you're showing them right. So I assume that that if we actually look at the hammer file like masterwise- and we look at the at the history of that one- probably someone removes.

A

A

Fixed display set basic.

A

Update styling for modal titles, okay, web shut cut the docs modal width.

A

Improve performance bar fix shortcuts.

A

People touch that file more often than I than I.

A

A

Sometimes you know.

A

Sometimes commit messages that would be helpful would be helpful.

A

I mean we could do a bisect and find out, but to I'm 90 percent concern uh sure that this is not used anymore yeah. I am too like we can search again, but there's no hidden here, there's like no class selection or anything right. I.

E

A

This is just an artifact from back when.

A

Okay: let's try it out the other way.

F

A

The element- don't you?

A

Yes, I have to create the element.

A

A

This is an interesting path down. History: hey! We just need 17 more weeks. If we do one every week.

A

I emerge, hey, look, it works nice.

B

It's a couple of unescaped template strings, but what why.

A

D

Because you didn't register glycogen, obviously.

A

I didn't import gear icon from gitlab ui here.

E

A

Oh this, this time I even should get hot module reloading. Does it even work with dynamic imports would be interesting to know?

A

Yes, it did kinda.

A

I mean this. This works. I mean.

D

uh You need to also use uh parentheses for control.

A

Character for control character. What do you mean.

D

uh For the computer property you're calling control character as a just a prop uh like you go down to the template. Oh you you! You did.

B

A no no go back.

D

A

I did a search and replace and replace it here. Yeah.

D

A

uh Command, I can make unicode.

A

C

A

uh By the way, uh control, for example, would be something that is translated in in german. uh We call it stoyrong strg, so like literally like control, but in german.

C

A

B

So maybe that should be a translation string as well, then.

A

But maybe in the future I mean to be honest: yeah uh yeah, I don't know it's like people. Oh wait, what's happening, why doesn't it work? Wait question mark.

A

um Also, the removal of itself will be funny right like here. We have the modal, we add the modal, so on toggle help toggle help, so this does modal modal, which we would not want to do like.

A

This is the only part of the logic. That's a bit. You know like wait. Why.

C

A

It working anymore: do we get like a compilation, error or something?

A

No, I mean it just worked a second ago. Didn't it.

B

Yeah not imagining it.

A

A

B

Still saying recompiling or is that uh I don't.

A

A

It's weird because I'm typing a question mark like toggle help.

B

Let's go to console.log and see what happens.

B

There's so many s's in that console.

C

C

A

A

I'm just going to close the other two tabs because probably takes more time than needed.

A

C

A

Oh, I have a bad feeling.

A

Like, let's go.

A

Wait shortcuts js.

A

Oh, this is bad.

A

I think this was just merged if page and page doesn't include page with custom shortcuts, okay, so this is the other way around, so projects groups show with which page are we on like.

A

Could have had to look into the let's: let's try it here.

A

Nothing is happening.

C

A

Am I somehow did.

B

You rebase recently or why would a recent change or change.

A

Broken I just I just pulled earlier, but it was just like working a second ago right, like.

B

So the binding itself just isn't working because we're checking the infinite toggle um that happens after this mousetrap find right.

A

Yeah initialized, let's just lock that cyber collapsed boobie. Whatever I mean it should tell us, say.

A

It's it's funny because it doesn't initialize the shortcuts on the project's show page.

A

So wait which one would be okay, like let's go to this page. This page is like projects.

B

Because why are we disabling the shortcuts on that page.

A

Yes, this is a really good question like okay, but it works and we have the shortcuts right. I don't know what the toggle is supposed to do if it's like supposed to.

A

I mean it's super weird: it's like hey.

B

Are these pages that have specific shortcuts related to them, and maybe we don't want to show a menu that might not encompass all the shortcuts that should, I don't know, maybe they have a different help menu that they're trying to bind.

A

Wait: let's, let's have a: let's, have a look, so this is the normal one right now, let's maybe go to.

A

ah So these shortcuts extend the normal shortcuts.

A

But that should mean like some are probably not doing, that right.

E

A

Shortcuts navigation shortcuts.

A

A

It's super weird like because what are the? What are the project? Specific shortcuts like global shortcuts project, gc navigation shortcuts navigation? So these are like those right shortcuts, navigation? Okay, where are they used?

A

These are used on oh wow, on a lot of pages.

A

Extends shortcuts, but if this is the constructor and it just extends the constructor.

A

And it's called it should do the thing right like it's calling super like what console log, what.

A

Like I think we just broke shortcuts in that other, mr, like, if I'm because creating a new issue should just be, I right, like hey.

A

Yeah index, where is this all like.

A

Project boards project commit show projects commits.

A

So, if we're now not importing the shortcuts.

A

What now it's doing it.

C

A

What shortcut navigation.

A

It's doing the thing, but it's not doing the.

A

A

This is like super weird shortcut: navigation from.

A

Shortcuts, like super weird, that it's not doing the thing anymore right, like mousetrap, find what.

B

Was the purpose of the change? That's that modified this behavior.

A

Oh yes, I know now, um so this change should be fine. You know what my assumption is. What's that no, it makes no sense that it's suddenly broken but like on.

A

A

So the purpose of this change is not to before it was importing shortcuts every time and just sometimes not initializing them right, and now it's not initializing them. You know what my assumption.

B

Actually, it's now.

A

A dynamic import that was happening.

B

Yes, who made this change? Was this one of tim's things? Yes,.

A

It should be fine, you know what my assumption, though, is like mousetrap.

A

ugh Expose why go away.

A

My assumption actually is that uh this change, together with that change, like an update and mousetrap, doesn't do the thing anymore. Let me just try out like one tiny, tiny thing like in the shortcuts. What happens if we are like.

E

E

A

My assumption is that the second mousetrap import actually overrides the first one.

F

Yeah, it's possible yeah! Well,.

B

I'm actually not sure, because unless the module is duplicated between chunks.

A

Which it could be in development mode right.

B

Yeah, that could be the hot module replacement might be messing with that too.

A

A

Like is this super confusing, like.

A

Mouse mouse trip bind like it's. It's clearly binding the thing right, yeah.

F

Anyway, maybe mousetrap has an api where we can just like. Have it print.

B

All the existing bindings or something we can do a sanity check there.

A

Now it's not working here anymore,.

A

Wait mousetrap we're now looking at mousetrap apis right, like mousetrap.

E

See here's the.

B

Api reference find online trigger add key codes. I don't know if.

F

B

A way to just doesn't look like there's a public api to list existing.

B

A

What do we have here bind handle key in it. Pause prototype trigger reset, stop callback target trigger unbind onpause. Like I mean what else did we change? Maybe one of my changes broke a thing or something right like.

B

It was working perfectly fine. Well, I guess I don't know, um I don't even remember the order of events.

B

B

That contribution graph looks really odd in dark mode.

A

Yeah, a lot of things look odd in dark mode, but why is it not working like makes no sense at all? It's even initializing the thing.

B

B

Are so are no keyboard bindings, working or just the help? One now.

A

No, I I mean nothing is.

A

A

Like ah I know why oh gosh, I'm so stupid what happened ah this is so stupid.

A

Shortcuts init toggle button shortcuts toggle shortcuts, enabled.

C

A

But wait if you have shortcuts enabled local storage usable on change, enable shortcuts disable shortcuts.

A

So if I disable shortcuts, I I disable shortcuts via the shortcuts thing yeah. So I probably need to go into settings or something and in that little toggle button on.

B

A

This is stupid.

A

Like storage, local storage, shortcuts, disabled delete shortcut.

C

A

Like how would you enable it again, if you don't know.

F

Yeah is there a setting in like that.

A

Yeah it works now, so I validated the thing that this actually works.

A

Oh gosh is that stupid? um Okay, now the question is like: how do we close the thing ourselves right.

B

Some shouting going on upstairs. um What do you mean close? The thing you mean the modal.

C

B

Does the x button work there.

A

No, it doesn't.

F

Should we just.

A

Should be just I mean really stupid idea, shortcuts help uh x, button button area label class data dismiss modal. I don't care about this stuff just be like type button. Ad click.

A

Close model, and then we just do a close model. You.

B

Want to make a method, not a computed property right.

A

Obviously, yeah.

A

Like this comma.

A

Methods- and we just do modal.

A

Document query.

E

C

C

A

What did I remove the class or something.

A

Wait document query: selector is null.

A

A

ah It worked needed to update uh okay.

A

Oh wow, this is funny.

A

uh We also need to do that same thing. Essentially, if shortcuts.

A

A

Maybe not the most efficient way to do it.

A

C

B

That's it gets the job done right.

A

Yeah- and I mean it's for the shortcuts- help right- it's not like hey. uh This needs to be like super. Whatever some follow-up should be.

B

Refactoring this to use gl model or something.

A

I don't know how hard it would be to use gl model.

C

A

Okay, now it doesn't show anything at all.

A

um Maybe because it's reloading but like gitlab ui gl model should be easy. Gitlab, your igl model get them.

E

Base this is like.

A

Now, it's not doing it, ah it's doing something. It just takes some time to remember.

A

uh Yeah be probably because of the async import right.

A

But yeah even then it's like a model default, so open model opened modal, so basic.

E

A

So do we do just this yeah probably like this right, like should be oh.

F

Yeah, how do you trigger opening the modal we're not relying on the bootstrap code anymore, right.

A

Basically, like we do it right now, but my question for the model would be like hey: how do we model okay, we're deprecating, more k, modal footer slot model, footer yeah?

A

I want this to be just like what what if I don't want to have a footer like.

F

Does it look like that was uh intention yeah? That was not.

A

Slots, it says we have, we have slots like modal header and close.

A

This is like really not well documented.

E

A

Say we need to look at the source.

A

If you need to to run run right, and we can end this and.

F

I can stick around for a little bit more. Do we want, I mean, do we want to do all this in one merge request, or should we just split it into two.

A

Yeah, I think we.

B

A

It in one merge request that depends on the the implementation right. It's like hey. Oh, we have a modal header slot.

A

And we have a modal footer slot ah v. If so, if we don't give any action.

B

Yeah so we're gonna have, to just add a modal footer slot and make it empty or something.

C

A

Gm motor right, okay, so gl model class model. I think we probably can remove most of these, I'm just going to move that to a comment for now. I don't care about these.

C

A

Yeah gl model and modal dialogue modal large. So are there like properties that we can use for this like b model.

E

B

Html and in the um live ui uh storybook documentation, maybe.

E

Like I don't know, what mobile dialogue is, is that a custom class that we made, I don't know.

A

Also seems weird because, like this is the whole, we probably can remove this one as well, probably need to remove a closing div.

A

And then we, our modal content. This also seems wrong, like modal content model header right, so I assume that this is diff class. No, so how do we do this? We do templ, oh gosh model content.

A

So we do template and then more.

A

A

Template and then.

A

Why is the other one a different slot like okay, see what happens now.

A

Tag template okay, missing required, prop modal id.

B

So we have it just set to id because it has to be modal id.

C

B

Just removed the regular id right, I'm assuming that that's what this does is pass it through.

C

C

Okay, wait what.

A

Is it now taking too long.

A

Too, does it do anything.

A

You know what we should do. um We should file and follow up for someone to convert it to gitlab model.

B

Sounds good to me.

A

ah Thanks, uh I don't know someone mentioned it before. I had this great idea. You know.

A

So why is this complaining style and styling is complaining here because they want a new line after semicolon, doesn't seem right for inline styles to me: okay, uh yes, so this otherwise wants to translate everything we remove the modal shortcuts right.

A

Maybe I don't know I am proposing that we just call it uh uh differently like what do you think about? Oh okay, help pages spec.

A

Okay, user uses shortcut spec.

A

Oh, oh, this is interesting, so this test will definitely fail. Can disable shortcuts from help menu model shortcuts, but.

B

Hidden yeah, I'm guessing really going to be some well. I assumed there would be some tests. We would have to fix.

F

Just based on this new implementation.

A

So maybe like this right, expect page not to have modal shortcuts because wait open modal shortcuts still in dom but hidden page refresh, yeah.

A

Open open modal shortcuts find body send key. What is this testing?

A

Can disable disabling shortcuts? ah Okay, ah remove item? Okay, open click, toggle button, ah close modal okay, so this is is okay. If we change this test, I believe, because.

A

Yeah, I think if we changed, this test should be.

A

C

F

A

Why is it trying to eyes uh yeah, okay cool, so the only thing that I've messed up is this button.

A

And I have an easy idea to get the correct classes back.

A

Class close, maybe we should add that.

A

A

What's the what's the correct for aria hidden, I'm.

B

Not sure um might just be yeah, sometimes you can just include it without a value, and I think that defaults to true or something like that, like disabled.

A

Yeah, so we now could go and remove the shortcuts.

A

Js.Harmel right, yes,.

A

And we can move and.

B

Remove all the console.logs that we added to.

A

uh Shortcuts harman right, we can remove this one right.

C

C

A

huh Just makes sense.

A

Okay, let's see sourcetree.

A

Okay, I think there's so this definitely makes sense. We are removing this.

A

We fixed the spec.

A

We these logs, are don't matter to us.

A

This all looks fine.

A

This all looks fine as well, it's just like the oh. We have translation.

B

Strings around the keyboard shortcuts.

F

Now, all of a sudden.

A

Yeah this is this is not right, so this is right.

A

This doesn't seem to be right right.

B

Yeah replace that I forgot about.

A

No, maybe I did some auto fixing with eslint that you know.

A

So this is definitely off as well like we can get rid of these. We don't want to translate single letters.

E

A

Okay, these are all keyboard shortcuts.

A

E

A

E

A

All of all of these are shortcuts.

A

Toggle help okay, so removing all of that makes sense right.

C

A

uh This we don't need this, we don't need.

A

This makes sense.

A

And here this also makes sense: okay,.

C

A

No, we don't want to change the yammer file. We just want to do this. This looks good, oh jeez,.

B

Oh everything got indented yeah, you removed some wrapper element or something.

C

A

uh Eslint yeah, I'm always running eslint, the fix which we don't want to do here right.

B

Does that honor the yes like disable comments.

A

Yes, I think I added it after the fix yeah, so so should probably be something like this. Let's run it.

C

A

I don't want to do the stupid hammer just called changes uh and this okay, I think.

B

This looks bad like.

A

Yeah, it isn't pretty, I ignore, I don't know, is.

B

It I don't know, I don't know I haven't. I haven't looked at our pretty very good one.

A

Yeah, so I think this this looks good to me.

B

It's like gamma's, not included in prettier, ignore, but we probably should add it.

C

A

um We should just use prettier for yama as well, uh because then at least we have consistent yamls, um but that would mean the backend.

A

People yeah, I think I think this looks about right, right, yeah, to be honest, like I'm a bit unhappy that we even use the gitlab icons here, I would just use like arrow ups from unicode, because then at least it matches, the style of you know is.

B

There a reason we can't.

A

No, the reason is uh I I want this review to be purely front-end review right.

A

My great keyboards help to view.

A

Modal shortcuts. I think the name is fine right, because I wanted actually wanted to change the name but yeah.

B

A

A

But how does one like in the spec so disabling from the help menu open modal shortcut keys.

A

Re-Enables shortcuts, how hub what click toggle button the thing.

E

A

How would one re-enable, if the thing is not working must well.

B

If it's, if the menu is already open, you can click the button again to toggle it open or on but yeah. Otherwise, I'm not.

F

A

Yeah, this is a bit. You know copy open modal from help.

B

Menu, so maybe you can open it, alternatively, by just going to help.

F

C

A

Gotcha so bin spring.

A

Copy path- and this is something we can do while oh pin spring aspect right- and this is then something we can do. Git.

C

A

A

Steady replace how of.

A

Keyboard help model to you: okay,.

F

Thanks, whatever enough changes.

A

Yes, I will write something currently, the.

A

Keyboard keyboard shortcut model is rendered via harmony and injected into the dom via.

A

Javascript evil. This is not good. This is rather inefficient.

A

In efficient and might even introduce.

E

E

Thus we move the static.

E

A

We got problems.

A

Okay, something failed. ah This is so helpless.

F

Too many sql queries. It.

A

Says wait user user shortcut can disable shortcuts can disable shortcuts from help menu open modal shortcuts, close modal.

A

ah This doesn't work right.

A

Wait, there's something we can double. So we do this, we disable it. We close it. We do this, nothing happens right. Does it like show an arrow or something it could.

B

Be that it still exists in the dom, even though you can't open it anymore,.

A

uh I mean, with close, we actually should be removing it, but you're right. Let's just check that in the body we don't have it anymore. It doesn't look like it so keyboard shortcuts uh document, query selector.

A

What was it modal shortcuts that sounds right? Okay, remove it close it null.

A

I mean if that error, ah internal server error. Okay, so I think this this thing was unrelated. This failure was maybe just because of startup, whatever.

A

A

Okay, get them off.

B

Server error looks like the javascript console output. Can you scroll up a little bit? Maybe there was an actual error.

A

B

Yes, um threshold exceeded error, maximum 100 queries yeah, so that doesn't look related either.

C

A

Just trying it again oops what did I do same arrow?

A

I mean, let's see if ci is green or not right,.

F

Yeah, oh, they need some coffee.

C

A

I think this doesn't need any screenshots, because it's just the same right right.

A

A

Cc, who was he uh jivanil david mike.

A

C

A

Front end: maintenance, uh technical debt- I don't know performance but yeah.

A

A

huh So this is unfortunate. We actually introduced more code than we've removed, stupid, auto formatting, I mean yeah. Obviously you already get double the lines with double the lines with uh with vue because of the closing tags right.

B

A

This is funny this is like yeah, okay, so this starts the search. That's interesting never knew about that shortcut. um Yeah cool um yeah, nice, not what we wanted to do, but actually good work. I'd say how was this actually working before? Did it really evaluate something.

F

Yeah global even entered all the javascript, and now that hamlet template and then it would just use axios to grab that and valid yeah.

C

B

That used to be how we would do the um commit history that would appear in the main repository view too. We would just pull something and like eval from from server-side generated javascript, and it was, I think, we've gotten rid of that now. Okay, that used to be everywhere.

A

Yeah, okay: I will stop the recording and share.