vertical nav bar issue about daily-apps

[saqibameen]

@idoshamun if this hasn't been resolved yet, I would love to contribute on this one.

[saqibameen]

Sure. I was about to ask. In my secondary screen it is constant to 48 I guess and then it's blank. So, I believe fixing it will be a good idea.

[idoshamun]

I believe you nailed it! You managed to go back in the git history and understand my deeds 💥
Let's create a test to cover this case so it won't happen again, what do you think?

[idoshamun]

Your new test should be placed here:
https://github.com/dailydotdev/daily-apps/blob/master/packages/extension/__tests__/Home.js
You can find there references for tests of the Home page.
I believe your test should check that more line numbers are added when more content is added to the feed. Does it make sense to you?

[saqibameen]

Yeah, it totally makes sense. I'll take a look at it and get back.

[saqibameen]

@idoshamun I have created PR where this issue is fixed. Let's collaborate there and move forward to add test and complete this.

[idoshamun]

@AM-77 thank you for the contribution and reporting this bug :)
If you are up to the challenge, we would love to get your contribution on this one

[idoshamun]

@saqibameen thank you! I would be great :)

[saqibameen]

Awesome! So, which way would you like me to proceed? Should we fix the sidebar or navbar?

[idoshamun]

Sidebar, please.
I also noticed that in all screen sizes the numbers in the side don't update.

[saqibameen]

I changed the position from absolute to fixed so it is fixed w.r.t the viewport and doesn't change on a scroll. But it leaves the space on top empty. How does it look to you?

I believe we need some sort of event handler so it gets a negative margin every time it's not in the default position. What do you say? If yes, what would it look like? @idoshamun

Take a look at the GIF.

[idoshamun]

Actually it was once worked perfectly fine. The expected behavior is that you will have the numbers keep running. This is why it should be absolute. I believe something messed up here:
https://github.com/dailydotdev/daily-apps/blob/master/packages/extension/src/routes/Home.vue#L208 due to changes in the dom structure of previous commits. Basically, I'm trying to calculate there how many numbers can be filled vertically and then actually adding them to the sidebar. Maybe you should dive there and try to see what changed and fix it. Or maybe it doesn't call this function anymore when the feed is updated.

[saqibameen]

@idoshamun thank you for the input. So, I tried to debug found out that the function updateLines() is not being called when the feed is updated.

It was supposed to be called under a watcher named posts().

daily-apps/packages/extension/src/routes/Home.vue

Lines 448 to 451 in 15bce77

	watch: {
	posts() {
	this.updateLines();
	},

I could not find any posts data. I looked into the commit history and found out that you probably created a Feed component and encapsulated posts into that. Or changed the store according to that.

If I change posts() to feed() in the watchers, and it works fine:

daily-apps/packages/extension/src/routes/Home.vue

Lines 448 to 451 in 15bce77

	watch: {
	posts() {
	this.updateLines();
	},

Here is the commit where posts is being removed from the DaHome.vue component.

8e3dca5#diff-28147bdc2c22676eb273ca31213a595eL409

Let me know if I'm approaching it the right way, I'll go ahead and create a PR.

[saqibameen]

Sure, can you please guide me about the tests in Daily and how'd this test look like?

[saqibameen]

I have spent quite some time on this test. I'm not well versed in testing, so tried to make sense of the existing tests. Below is what I understood, let me know if I am headed in the right direction.

• Currently, the posts array is empty in the state. So, the feed is not being rendered.
• We'd probably first need to add some items to the posts array in the state. Then add some more items OR try to somehow trigger the fetchNextFeedPage().

What I am understanding is, we need to somehow trigger fetchNextFeedPage() method which is triggered on the scroll. I am not sure in testing if we need to provide its implementation in Store or how that's going to work?

[idoshamun]

Actually updateLines is called upon any change to the posts object in the store. You don't have to actually call fetchNextFeedPage but rather add more posts. Once the posts object changes the updateLines will be called.

[idoshamun]

No longer relevant as we updated the design. Thanks for everything :)