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

keep height of persistent language tabs constant #1227

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

titusfortner
Copy link

@titusfortner titusfortner commented Sep 5, 2022

Updates

Re-implemented with a simpler approach.

  1. Instead of simulating clicks, this calculates the maximum height by just briefly displaying the tab, measuring its height, and then hiding it again
  2. This code applies to all tabs, not just persistent ones
  3. I set the size on the tab-pane elements so that the extra space would have the border instead of being weird empty space
  4. This is a nested loop instead of storing values in a giant hash, but I think the performance is good enough that it does not matter?
  5. Might want to add a toggle to turn it off? Maybe persist=disabled turns it off?
  6. I'm sure there are ways to make this prettier

A good example to see this working: https://deploy-preview-1530--selenium-dev.netlify.app/documentation/webdriver/interactions/windows/#print-page

Note that this page was the one that was giving problems with the original implementation here; still have no idea what wasn't working with that one.

Again, not a JS dev, but this seems to work as desired.

I haven't merged it into my project yet (SeleniumHQ/seleniumhq.github.io#1157), but you can see an example by going here and clicking the "Kotlin" tab:

https://deploy-preview-1157--jovial-austin-42fe02.netlify.app/documentation/webdriver/actions_api/mouse/#drag-and-drop-by-offset

Not sure if you prefer an additional toggle added for this behavior & I know the shortcode docs section would need to be updated, either way.

@LisaFC
Copy link
Collaborator

LisaFC commented Sep 13, 2022

Geri, I know you were looking at some tabs stuff, can you take a look?

@LisaFC LisaFC requested review from geriom and deining September 13, 2022 10:59
@geriom
Copy link
Collaborator

geriom commented Sep 14, 2022

Thank you @titusfortner, I'm looking into this later today.

@titusfortner
Copy link
Author

Thanks! This is merged in Selenium docs now.

You can see this example: https://www.selenium.dev/documentation/webdriver/actions_api/mouse/?language=javascript#drag-and-drop-by-offset

Clicking the different tabs keeps the same position, even though the previous tabs are very different heights.

@geriom
Copy link
Collaborator

geriom commented Sep 15, 2022

@deining Since you are the main contributor to this feature. Can you please take a look when you have a change?

@titusfortner
Copy link
Author

Well, something is wrong with this code after all. Clicking the tabs and measuring the heights isn't always giving accurate heights and I'm not sure why. If I throw a debug point and execute the code in the console it gives a different result from when it is executed in the js file. I have no idea if it's in a different context or what is going on. :(

@titusfortner
Copy link
Author

@AryanP45 you can see from this example: https://deploy-preview-1157--jovial-austin-42fe02.netlify.app/documentation/webdriver/browser/windows/?language=python#create-new-window-or-new-tab-and-switch

There's something wrong in the calculations, and I just couldn't figure out the issue. For some reason clicking the tab in the JS and calculating the height was giving me different value than when I ran it in the console. I don't know enough JS to know how to fix it.

Full conversation is here: SeleniumHQ/seleniumhq.github.io#1167

Would be really great if someone could at least figure out *why the JS isn't determining the correct tab-content heights when executing. I put in a debug point to ensure it isn't a race condition. Just no idea why it's happening. Can't hope to do a workaround without that.

@chalin
Copy link
Collaborator

chalin commented Oct 7, 2022

  • @titusfortner - which version of Docsy are you using? AFAIR, there have been some updates to the tabpane/tab shortcodes. From looking at your site I get the impression that the version of Docsy that you're using doesn't have the latest fixes. Thanks for all the context you provided.
  • @deining - gentle ping, in case you have something to add.

@chalin
Copy link
Collaborator

chalin commented Oct 7, 2022

I can't reproduce the problem reported in SeleniumHQ/seleniumhq.github.io#1167 under macOS Chrome. @geriom, can you reproduce it? (Works fine under macOS Firefox too.)

@geriom
Copy link
Collaborator

geriom commented Oct 14, 2022

I can't reproduce the problem reported in SeleniumHQ/seleniumhq.github.io#1167 under macOS Chrome. @geriom, can you reproduce it? (Works fine under macOS Firefox too.)

@chalin, same, I can't reproduce it either under Linux Chrome and Firefox.

Edit: Nevermind, I was able to reproduce it once, after clicking the right hand side link to the header.
wrongTabpane

I can't reproduce it again. The page is working fine except for that one time.

@titusfortner
Copy link
Author

Huh, when it happens for me it's been consistent, it's just not obvious to me why it happens in one place and not another. Seems to be the tabs with the most height? We could set it not to overwrite height value if the max height is over a certain amount, but those are the tabs that end up causing the most problems. :)

@titusfortner
Copy link
Author

Erf, but for some reason the tab height does not work when I combine it with #1202
So, this one might need more work again.

@titusfortner
Copy link
Author

I was just combining them incorrectly.

This example includes code from both of my Docsy PRs:
https://deploy-preview-1530--selenium-dev.netlify.app/documentation/webdriver/interactions/windows/?tab=python#execute-script

@chalin chalin modified the milestones: 24Q1, 24Q2 Jan 11, 2024
@chalin chalin modified the milestones: 24Q2, 24Q3 Apr 2, 2024
@chalin chalin modified the milestones: 24Q3, 24Q4 Jul 2, 2024
@chalin chalin modified the milestones: 24Q4, 25Q1 Oct 3, 2024
@chalin chalin modified the milestones: 25Q1, 25Q2 Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants