docs icon indicating copy to clipboard operation
docs copied to clipboard

[WEB-3923] - Fix Language dropdown positioning for single item

Open aralovelace opened this issue 1 year ago • 1 comments

Description

WEB-3923

Review

Refactor the LanguageDropdownSelector and simplify the option display Label and should use flex even if it has no label

Also update the test after the Javascript version in the textile has been updated

Instructions on how to review the PR.

  • Single Dropdown - https://ably-docs-web-3923-7sarllfa4g8.herokuapp.com/docs/chat/connect
  • Multiple Dropdown - https://ably-docs-web-3923-7sarllfa4g8.herokuapp.com/docs/getting-started/quickstart?lang=javascript

aralovelace avatar Aug 16 '24 14:08 aralovelace

That is such a nice and simple fix, however I wonder about the UX:

Monosnap Connections 2024-08-16 16-49-29

@m-hulbert wdyt?

Yeah, if we can have the language name as well for consistency I think that would be good, please @aralovelace 🙂

m-hulbert avatar Aug 16 '24 16:08 m-hulbert

@m-hulbert @kennethkalmer The text "Javascript" is not displaying at the single drop-down because there's only one option in the drop-down box. Based on how the language dropdown component is being designed, I suggest, that if there is only 1 language in the dropdown we will disable it. There is no point in clicking the dropdown if it's only 1 and it is the default

aralovelace avatar Aug 21 '24 10:08 aralovelace

Looks okay to me, but will wait to stamp this until the most recent comment above is resolved

jamiehenson avatar Aug 21 '24 12:08 jamiehenson

@m-hulbert @kennethkalmer The text "Javascript" is not displaying at the single drop-down because there's only one option in the drop-down box. Based on how the language dropdown component is being designed, I suggest, that if there is only 1 language in the dropdown we will disable it. There is no point in clicking the dropdown if it's only 1 and it is the default

I have no objections to that @aralovelace, though I think if we disable the dropdown (as I can see you've done), then we should probably hide the chevron too.

m-hulbert avatar Aug 21 '24 13:08 m-hulbert

@m-hulbert @kennethkalmer The text "Javascript" is not displaying at the single drop-down because there's only one option in the drop-down box. Based on how the language dropdown component is being designed, I suggest, that if there is only 1 language in the dropdown we will disable it. There is no point in clicking the dropdown if it's only 1 and it is the default

I have no objections to that @aralovelace, though I think if we disable the dropdown (as I can see you've done), then we should probably hide the chevron too.

@m-hulbert thanks for checking. The thing is it's part of the dropdown so we can't remove it. What we can do is just display it in a container with the same styling as the dropdown. if more than one language then use the dropdown, what do you think? or this is much easier to change the indicator color to grey if it's disabled like this:

image

aralovelace avatar Aug 21 '24 14:08 aralovelace

thanks @m-hulbert

Can I get a final code review from either @kennethkalmer or @jamiehenson please thanks

aralovelace avatar Aug 22 '24 10:08 aralovelace