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

send window width in formplayer query #34832

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

Robert-Costello
Copy link
Contributor

@Robert-Costello Robert-Costello commented Jun 27, 2024

Product Description

This is part of the body of work to allow showing different content (forms, menus) depending on the user's screen size.
This example below shows a menu with a display condition that check that the windowWidth is >= 992.
In a real-world implementation there would be another for whose display condition would check that the windowWidth was < 992. That way the menu and forms that are optimized for a particular screen size would be displayed to the user.
content_based_on_window_width

Technical Summary

Jira ticket

Feature Flag

no

Safety Assurance

Safety story

this simply sends an additional query param to Formplayer, so is unlikely to cause any issues.
tested locally and will go through QA along with the FP component.

Automated test coverage

no

QA Plan

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@dimagimon dimagimon added the Risk: Medium Change affects files that have been flagged as medium risk. label Jun 27, 2024
@Robert-Costello Robert-Costello marked this pull request as ready for review June 27, 2024 16:30
Copy link
Contributor

@MartinRiese MartinRiese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good otherwise

@@ -184,6 +185,7 @@ hqDefine("cloudcare/js/formplayer/menus/api", [
"isShortDetail": params.isShortDetail,
"isRefreshCaseSearch": params.isRefreshCaseSearch,
"requestInitiatedByTag": params.requestInitiatedByTag,
"screen_size": screenSize,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would call it windowWidth to make it clear which dimension it is and also use camelCase for Json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windowWidth is a good idea. When I started on this it wasn't clear exactly what the value was going to be, but since it turned out to be the actual width, as opposed to a boolean or something else, windowWidth makes sense.

As far as the snake_case goes, I was following what seemed like a standard pattern in FP's SessionNavigationBean which extends InstallRequestBean which extends AuthenticatedRequestBean. That turns out to not be consistent when you look at restoreAsCaseId in AuthenticatedRequestBean so I'm not sure it matters too much. camelCase also doesn't break anything in FP so it seems fine to use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just looking at the line above as reference.

@Robert-Costello Robert-Costello added QA Passed and removed awaiting QA QA in progress. Do not merge labels Jul 8, 2024
@Robert-Costello Robert-Costello merged commit 93147ad into master Aug 5, 2024
12 checks passed
@Robert-Costello Robert-Costello deleted the rc/send-screen-size-to-fp branch August 5, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Passed Risk: Medium Change affects files that have been flagged as medium risk.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants