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

CSS changes for responsive design #1646

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

zedhaque
Copy link

@zedhaque zedhaque commented May 22, 2024

Purpose

This PR introduces changes in the CSS files to enhance responsiveness in the chat app, supporting the most common breakpoints. Additionally, it transitions styling units from pixels to rem/em for better scalability across devices. This PR does not include the implementation of a sticky header or any other changes requiring modifications to .tsx files; those will be addressed in a subsequent PR.

#1458

Does this introduce a breaking change?

When developers merge from main and run the server, azd up, or azd deploy, will this produce an error?
If you're not sure, try it out on an old environment.

[ ] Yes
[X] No

Does this require changes to learn.microsoft.com docs?

This repository is referenced by this tutorial
which includes deployment, settings and usage instructions. If text or screenshot need to change in the tutorial,
check the box below and notify the tutorial author. A Microsoft employee can do this for you if you're an external contributor.

[ ] Yes
[X] No

Type of change

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

Code quality checklist

See CONTRIBUTING.md for more details.

  • The current tests all pass (python -m pytest).
  • I added tests that prove my fix is effective or that my feature works
  • I ran python -m pytest --cov to verify 100% coverage of added lines
  • I ran python -m mypy to check for type errors
  • I either used the pre-commit hooks or ran ruff and black manually on my code.

@zedhaque
Copy link
Author

zedhaque commented May 22, 2024 via email

@pamelafox
Copy link
Collaborator

Thanks so much @zedhaque ! I'll take a look at this soon. I might try to run our Playwright tests against it, since it's in theory easy to tell Playwright to test mobile size configurations.

@zedhaque
Copy link
Author

@pamelafox - reviewing this. I haven’t tried playwright before. I will see if this is something I can also implement on my end. There are still some known issues - I am trying to fix. Just let me know your findings and will update accordingly.

@pamelafox
Copy link
Collaborator

@zedhaque I've run prettier on the files, and then used Playwright to run our two main tests for the Chat and the Ask tab at the various breakpoints used in the media queries. The Chat tests pass (and they failed before!), but the Ask tests break due to the "Ask" link being completely hidden in the header. Is it possible to un-hide that in this PR, or does that require the larger fix? If it's already the case in the main branch that it's hidden, I can just remove the viewport tests from the Ask endpoint and you can add those in the next PR.

@zedhaque
Copy link
Author

zedhaque commented Jun 7, 2024

Thanks @pamelafox - I will update the branch and unhide the link and update you on this thread this weekend.

@zedhaque
Copy link
Author

Screenshot 2024-06-13 at 10 28 26 AM

@pamelafox - I ultimately implemented a hamburger menu, as you suggested earlier. It's expandable, featuring options like "Chat with HR" and "Chat with JIRA". I made adjustments to the layout.tsx file to accommodate this. I have a couple of questions:

  • Are you in favor of sticking with the hamburger menu?
  • I propose replacing the current title with "Azure OpenAI + AI Search" and relocating the existing content accordingly.
  • Regarding the GitHub logo and link, should we remove them entirely or place them alongside the new title?
    Do you have any other changes in mind that you'd like me to incorporate?

Thanks.

@pamelafox
Copy link
Collaborator

  • I think the hamburger menu is the only way to reliably fit the options at low sizes. The world has collectively agreed.
  • Yes, let's just use "Azure OpenAI + AI Search" and drop the other one.
  • I think we can drop GitHub logo/link.

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.

None yet

2 participants