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

Investigate KSelect’s dropdown being displayed above the input in some cases even when there is enough space below #690

Open
MisRob opened this issue Jul 26, 2024 · 24 comments
Labels
Component: KSelect P2 - normal Priority: Nice to have

Comments

@MisRob
Copy link
Member

MisRob commented Jul 26, 2024

🌱 Are you new to the codebase? Welcome! Please see the contributing guidelines.

Summary

When there is enough space, KSelect's dropdown should be displayed below the input. However, there are several places in Kolibri where the dropdown is displayed above the input, even though there's enough space below. One example from Kolibri 0.17.0a0.dev0+git.123.g5cd1f6aa:

Coach → Plan → Lessons

This issue is present in other selects in Plan and Reports.

Interestingly, in a very similar place in Facility → Users, it works as expected:

The main culprit seems to be the bottom positioning set to 62px on .ui-select-dropdown. After commenting it out, the dropdown shows correctly in the Coach:

The bottom value is calculated in the calculateSpaceBelow method:

calculateSpaceBelow() {
// Get the height of element
const buttonHeight = this.$el.getBoundingClientRect().height;
// Get the position of the element relative to the viewport
const buttonPosition = this.$el.getBoundingClientRect().top;
// Check if there is enough space below element
// and update the "dropdownButtonBottom" data property accordingly
const notEnoughSpaceBelow =
buttonPosition > this.maxDropdownHeight &&
this.scrollableAncestor.offsetHeight - buttonPosition <
buttonHeight + this.maxDropdownHeight;
this.dropdownButtonBottom = notEnoughSpaceBelow ? buttonHeight + 'px' : 'auto';
},

The goal of this issue is to find out why exactly the calculation doesn't work well for places in the Coach (or check whether it’s overwritten from somewhere else, even though that’s rather unlikely). After we know more, we can determine the next steps and see if the calculation can be optimized.

References

Originally reported in learningequality/kolibri#12079 (not related to the refactor being tested there - the bug was present before).

@Sahil-Sinha-11
Copy link
Contributor

Hey @MisRob I was planning to work on this issue , so I was trying to reproduce the 1st img coach->plan->lesson but when I signed up in Kolibri I was signed up as a learner, could you please help me here. 😓

@AllanOXDi
Copy link
Member

AllanOXDi commented Oct 28, 2024

Hi @Sahil-Sinha-11 this could be that you are signing in with a learner account you created instead. You need to create a admin or coach account and log in

@Sahil-Sinha-11
Copy link
Contributor

Hey @AllanOXDi during creation of an account I dont see an option to make a profile as a coach, I am automatically assigned Learner.

@MisRob
Copy link
Member Author

MisRob commented Nov 8, 2024

@Sahil-Sinha-11 When setting up Kolibri, you created admin account in the setup wizard I assume? Can you use the admin account to login? That should make this place accessible.

@MisRob
Copy link
Member Author

MisRob commented Nov 8, 2024

@Sahil-Sinha-11 Also note there's more ways to setup Kolibri, some of them not having Coach features. I recommend you use "Group learning" -> "Full Device". More here https://kolibri.readthedocs.io/en/latest/install/initial_setup.html?highlight=setup#group-learning. Let us know if this helps. If you can't still access Coach, please post the recording or a list of steps you did.

@MisRob
Copy link
Member Author

MisRob commented Nov 19, 2024

@Sahil-Sinha-11 I've just noticed we forgot to make an assignment - should we do so?

@Sahil-Sinha-11
Copy link
Contributor

Hey @MisRob, I am almost done with issue #588 and waiting for it to be merged. As soon as it is done, I will start working on this one. You can assign me this though 🤗. Also, my exams are going on so I will be starting around 28th, sorry for the delay 😅.

@MisRob
Copy link
Member Author

MisRob commented Nov 19, 2024

No worries, absolutely no pressure! Thank you. Good luck with exams :)

@Sahil-Sinha-11
Copy link
Contributor

Hey @MisRob, how do I produce this locally? Is it in the kolibri ? I analysed the Kselect file and the calculations seemed fine when I logged the data, I want to analysis this as well for further understanding.
Image
I think there is some problem in the bottom CSS attribute.
Image

@MisRob
Copy link
Member Author

MisRob commented Nov 29, 2024

Hi @Sahil-Sinha-11, yes, this would be best reproduced and investigated in Kolibri in some places since it seems to be specific to them. One of them is mentioned in the "Summary". Let us know if you found some of them.

@Sahil-Sinha-11
Copy link
Contributor

Hello @MisRob, I am not able to run the kolibri locally, I tried to use docker but on the port mapping it said "empty", I attempted to follow the guidelines in kolibri but I got overwhelmed, would you please help me, I am sorry for the trouble 😓.

  1. I cloned kolibri repo
  2. I ran docker-compose up -d [ I guess it made an image for Postgresql ]
    This is what my terminal looks like,
    Image

@akolson
Copy link
Member

akolson commented Nov 29, 2024

Hi @Sahil-Sinha-11. You need to use the make .... command that takes care of the setup for you.

In the root of the kolibri project, run the following commands in sequential order

  1. make docker-clean
  2. make docker-build-base
  3. make docker-devserver
  1. is optional but good in cases where you need to start out on a clean slate, 2) is for first time runs and you don't need to run it again on subsequent runs, and 3) is what starts the server that you should be able to access via your browser at http://localhost:8000.

@Sahil-Sinha-11
Copy link
Contributor

Thanks a lot, @akolson, it worked.

@Sahil-Sinha-11
Copy link
Contributor

https://github.com/user-attachments/assets/63d48c0f-43ce-4dd2-803c-2d1171e8501d
Hey @MisRob, I think there is some problem with spacing as I observed above.

I Investigated some places in Kolibri I think the problem is that when the component is just below kselect it's not able to calculate the maxDropdownHeight correctly but if there is a kind of row space it works fine
https://github.com/user-attachments/assets/5bdae448-2c77-42f4-b450-ca242be0051a
Could you help me determine if I am going in the right direction and how to proceed here?

@MisRob
Copy link
Member Author

MisRob commented Dec 2, 2024

Hi @Sahil-Sinha-11, thank you for investigating!

To confirm I can understand, from the recording it seems that in this particular place the dropdown is position correctly only when we remove top margin 24px from the container, right?

That could give us hint. Next steps could be to debug step by step calculateSpaceBelow and related and compare the two cases where margin is or is not applied. To understand exactly where margin value is taken into account in the calculations.

@Sahil-Sinha-11
Copy link
Contributor

Hey @MisRob, when I make changes it does not reflect on the local host for example
like on commenting the warning section the local host is still same

Image

Image

can you please help me here?
moreover, on investigating again I guess the calculation is triggered by some element, as you highlighted removing the "bottom:62.5px" kselect works fine, this 62.5 is the button height!

@MisRob
Copy link
Member Author

MisRob commented Dec 4, 2024

Hi @Sahil-Sinha-11, how do you run your Kolibri development server? Are you following https://kolibri-dev.readthedocs.io/en/develop/getting_started.html#running-the-server?

@Sahil-Sinha-11
Copy link
Contributor

Hey @MisRob I was using make docker-devserver... I just followed the steps again and it works now, Thank you.

@Sahil-Sinha-11
Copy link
Contributor

Hey @MisRob, I did some investigations,

  1. Logged the calculated portion which seemed fine, If there is an issue it may be due to getBoundClientRect(). function which returns the smallest rectangle that contains the element.

  2. Image

  3. I tried to log the heights in kolibri but the kselect use there doesn't take ref as a prop so I was stuck there and couldn't log it .

Could you please guide me on how to proceed here? I would really appreciate your assistance.

@MisRob
Copy link
Member Author

MisRob commented Dec 5, 2024

Thanks you @Sahil-Sinha-11, I think these are all new and valuable insights. Unfortunately I have no idea myself on how to best proceed :)

@MisRob
Copy link
Member Author

MisRob commented Dec 5, 2024

the kselect use there doesn't take ref as a prop so I was stuck there and couldn't log it

@Sahil-Sinha-11 This seems like the only area I could be of help. Would you please elaborate a bit and reference some logging code, for example in a working branch, so I can understand better what was the issue?

@Sahil-Sinha-11
Copy link
Contributor

the kselect use there doesn't take ref as a prop so I was stuck there and couldn't log it

@Sahil-Sinha-11 This seems like the only area I could be of help. Would you please elaborate a bit and reference some logging code, for example in a working branch, so I can understand better what was the issue?

@MisRob I tried like this but on the console, it says it cannot read properties of null
Image
Image
Image

@Sahil-Sinha-11
Copy link
Contributor

@MisRob a temporary solution might be that we set bottom as auto for now, but that wouldn't be a good solution 😅.
For now, may I work on other issues?

@MisRob
Copy link
Member Author

MisRob commented Dec 6, 2024

@Sahil-Sinha-11

I tried like this but on the console, it says it cannot read properties of null

I think I'd need to see this in a commit to get a full picture. Perhaps you've already tried some of these, just some ideas I got:

  • You may try console.log all other variables you added at first, such as selectElement, etc. to see what information is missing exactly before the error gets triggered
  • If the problem is reactivity, e.g. the value is being only logged once (while still returning null), generally console.log can be done watchers can be used to get a full picture of reactive values

a temporary solution might be that we set bottom as auto for now, but that wouldn't be a good solution

Yes, that may not be ideal because we'd lose auto-adjusting if we did this KDS side. However, if we don't find a reasonable fix, we may actually consider overriding from Kolibri at those few particular places if it feels important. Thanks for the idea!

For now, may I work on other issues?

Of course! Thanks a lot for digging into this, it's a nuanced one. I hope you find some fun restful issue now :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: KSelect P2 - normal Priority: Nice to have
Projects
None yet
Development

No branches or pull requests

4 participants