Service catalog item page #535

gosiexon-zen · 2024-10-24T11:44:47Z

Description

Added view for service catalog single item. For now showing only Title and description and button for submmiting a request (without any action). Data is mocked for now as backend is still in development and we are merging into feature branch not master.

Implement rendering the Service Catalog Item Name and description

Screenshots

Collapsible description:
https://github.com/user-attachments/assets/2e672b5e-0bb3-4243-a879-d2358666ab0d

With button:

For small screens:

Checklist

📗 all commit messages follow the conventional commits standard
⬅️ changes are compatible with RTL direction
♿ Changes to the UI are tested for accessibility and compliant with WCAG 2.1.
📝 changes are tested in Chrome, Firefox, Safari and Edge
📱 changes are responsive and tested in mobile
👍 PR is approved by @zendesk/vikings

pfemo · 2024-10-24T14:37:48Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+        </ToggleButton>
+      </LeftColumn>
+      <RightColumn>
+        {/* Placeholder for Request a service button */}


Why not add the request button as well in this PR and have it be a dull button?

Actually we can add it 😄 we have another Jira for that, we usually split view for for different functionalities/parts and create tasks for that.

Ah okay; totally up to you. I just wondered as it seemed like the only thing missing it might have made sense to just include it to reduce overhead of reviews, opening PRs etc. but I'll let you decide what level of separation you prefer 🙌

Right, I added it and adjusted a styles a bit but it still needs some adjustments for mobile view.

kubraokcu · 2024-10-24T15:07:08Z

src/modules/service-catalog/translations/en-us.yml

+  - translation:
+      key: "service-catalog.item.read-more"
+      title: "Button to show whole description of the service catalog item"
+      screenshot: "https://drive.google.com/file/d/1pXglNRRqa23LoA7CuZOiAKQAuwkRKFqN/view?usp=drive_link"


Seems like there's an issue with the screenshot 😄

Thanks, I fixed the screenshots.

Fredx87 · 2024-10-24T15:49:31Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+  text-overflow: ellipsis;
+  width: 100%;


We don't need these 2 lines

yep, removed

Fredx87 · 2024-10-24T16:03:18Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+  return (
+    <Container>
+      <LeftColumn>
+        <ItemTitle>{serviceCatalogItem.name}</ItemTitle>


I would set tag="h1" for A11Y

Ok, Changed

Fredx87 · 2024-10-24T16:05:28Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+const ToggleButton = styled(Button)`
+  margin-top: ${(props) => props.theme.space.sm};
+  color: ${(props) => getColorV8("blue", 600, props.theme)};
+  font-size: ${(props) => props.theme.fontSizes.md};
+  &:hover {
+    text-decoration: none;
+    color: ${(props) => getColorV8("blue", 600, props.theme)};
+  }
+`;


I don't think we should force a blue color as in the design, but let Garden use the theme link color, which we are setting here:

copenhagen_theme/src/modules/shared/garden-theme/createTheme.ts

Line 27 in 7a42141

"buttons.anchor": css`

That's true, I followed the design but Guide has it's own theme so I removed the color property here.

Fredx87 · 2024-10-24T16:12:59Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+  const [isExpanded, setIsExpanded] = useState<boolean>(false);
+  const { t } = useTranslation();
+
+  const toogleDescription = () => {


typo: toogle

thanks, changed

Fredx87 · 2024-10-24T16:16:30Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+        <CollapsibleDescription expanded={isExpanded}>
+          {serviceCatalogItem.description}
+        </CollapsibleDescription>
+        <ToggleButton isLink onClick={toogleDescription}>


I think we should hide this button for screen readers with aria-hidden="true". If the screen reader already reads all the text even if it is clamped to 3 lines, it doesn't make sense to have this button for screen readers. We need to check if the description is red entirely first

Right, screen reader reads all of the description also the hidden part. So I added aria-hidden

Fredx87 · 2024-10-25T08:04:04Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+const Container = styled.div`
+  display: flex;
+  flex-direction: row;
+  justify-content: space-between;
+  gap: 120px;
+
+  @media (max-width: 768px) {
+    flex-direction: column;
+  }
+`;


Are we sure we want this behavior for mobile? Just flipping the flex-direction and keeping the 120px gap? From the design, it seems that on mobile the button is a sticky footer, but I am not sure.

I also don't like we are setting the gap to an hardcoded 120px value, but if that is the value we don't have alternatives

I removed the 120px value and changed it to space.xl and also added this value to the right and left margin property of the columns.

I changed a bit style for mobile but I will work more on that. I see that we have some global css properties for footer and container and this sticky button doesn't look like the one on the design.

Fredx87 · 2024-10-25T08:10:36Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+  justify-content: space-between;
+  gap: 120px;
+
+  @media (max-width: 768px) {


I would not hardcode the breakpoint value but read it from the theme ( p.theme.breakpoints.md) or use the mediaquery utility

Thanks, changed to the theme variable

Fredx87 · 2024-10-25T08:16:42Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+  border-bottom: ${(props) => props.theme.borders.sm}
+    ${(props) => getColorV8("grey", 300, props.theme)};
+  padding-bottom: ${(props) => props.theme.space.lg};


Are we sure that we always want to show a bottom border even if no options are available? I think the grey line is more like a separator between the description and the options, but we don't have a design for a page without options.

In any case these border and padding should not be set on the whole column but only on the first part, but it is ok to do it in the PR where we will introduce the options

Ok I will work more on that in the PR with form fields

kubraokcu

👍🏼🌐

Fredx87

Added a comment. For the mobile styles, it is ok to revisit them in the PR with the ticket fields

Fredx87 · 2024-10-29T15:13:15Z

src/modules/service-catalog/ServiceCatalogItemPage.tsx

+  @media (max-width: ${(props) => props.theme.breakpoints.md}) {
+    position: sticky;
+    bottom: 0;
+    background: white; /* Ensure the button doesn't overlap content */


We should use props.theme.colors.background, the background could be different than white

gosiexon-zen requested review from a team as code owners October 24, 2024 11:44

pfemo reviewed Oct 24, 2024

View reviewed changes

kubraokcu reviewed Oct 24, 2024

View reviewed changes

kubraokcu added the g11n-commented Commented by Globalization label label Oct 24, 2024

Fredx87 reviewed Oct 25, 2024

View reviewed changes

gosiexon-zen force-pushed the mbien/service-catalog-item-page branch from 7a42141 to 48d72ff Compare October 29, 2024 13:50

gosiexon-zen requested review from Fredx87, kubraokcu and pfemo October 29, 2024 13:51

kubraokcu approved these changes Oct 29, 2024

View reviewed changes

kubraokcu added g11n-approved Commented by Globalization label and removed g11n-commented Commented by Globalization label labels Oct 29, 2024

gosiexon-zen force-pushed the mbien/service-catalog-item-page branch from 48d72ff to 1804f84 Compare October 29, 2024 14:10

Fredx87 reviewed Oct 29, 2024

View reviewed changes

feat: added service catalog item page

482b957

gosiexon-zen force-pushed the mbien/service-catalog-item-page branch from 1804f84 to 482b957 Compare October 30, 2024 12:39

Fredx87 approved these changes Oct 30, 2024

View reviewed changes

gosiexon-zen merged commit 298585b into service-catalog-eap Oct 30, 2024
5 checks passed

gosiexon-zen deleted the mbien/service-catalog-item-page branch October 30, 2024 13:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Service catalog item page #535

Service catalog item page #535

gosiexon-zen commented Oct 24, 2024 •

edited

Loading

pfemo Oct 24, 2024 •

edited

Loading

gosiexon-zen Oct 24, 2024

pfemo Oct 25, 2024

gosiexon-zen Oct 29, 2024

kubraokcu Oct 24, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 24, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 24, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 24, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 24, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 24, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 25, 2024

gosiexon-zen Oct 29, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 25, 2024

gosiexon-zen Oct 29, 2024

Fredx87 Oct 25, 2024

gosiexon-zen Oct 29, 2024

kubraokcu left a comment

Fredx87 left a comment

Fredx87 Oct 29, 2024

Service catalog item page #535

Service catalog item page #535

Conversation

gosiexon-zen commented Oct 24, 2024 • edited Loading

Description

Screenshots

Checklist

pfemo Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kubraokcu left a comment

Choose a reason for hiding this comment

Fredx87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gosiexon-zen commented Oct 24, 2024 •

edited

Loading

pfemo Oct 24, 2024 •

edited

Loading