-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat(requestoverview): add link to staging/cms #1161
Conversation
src/layouts/ReviewRequest/components/RequestOverview/RequestOverview.tsx
Outdated
Show resolved
Hide resolved
src/layouts/ReviewRequest/components/RequestOverview/RequestOverview.tsx
Outdated
Show resolved
Hide resolved
This pull request has been stale for more than 30 days! Could someone please take a look at it @isomerpages/iso-engineers |
header: () => ( | ||
<Th borderBottom="1px solid" borderColor="gray.100" w="10rem" /> | ||
), | ||
cell: ({ row }) => ( | ||
<HStack spacing="0.25rem"> | ||
{allowEditing && ( | ||
<Link href="www.google.com"> | ||
<Link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seaerchin lgtm, could you add a screenshot of how this looks like on the UI or add steps to test this locally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a story written for this so it should show inside the storybook! can get to it via chromatic below (UI Tests/UI Review)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chromatic and other CICD builds are failing, may I trouble you to look into them?
@@ -239,34 +239,30 @@ export const MOCK_SITE_DASHBOARD_INFO: SiteDashboardInfo = { | |||
|
|||
export const MOCK_ITEMS: EditedItemProps[] = [ | |||
{ | |||
type: ["page"], | |||
type: "page", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice if this is a TS type rather than string
This pull request has been stale for more than 30 days! Could someone please take a look at it @isomerpages/iso-engineers |
7bf1cf9
to
4e2f6e0
Compare
!run e2e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline,
- We should confirm the behaviour of moving files - currently the change only shows up as a new page instead of both a delete and an add
- Confirm behaviour of delete as well - currently it shows up as a file which cannot be viewed on staging/cms (which makes sense, because it's not available anymore), but is this intuitive to users
Approving first because these are not blocking, but it is behaviour that we should confirm with design!
filed as IS-229 |
This pull request has been stale for more than 30 days! Could someone please take a look at it @isomerpages/iso-engineers |
This pull request has been stale for more than 30 days! Could someone please take a look at it @isomerpages/iso-engineers |
corresponding BE PR here
Problem
This adds the link in the review request to either the staging site or the cms site
Closes IS-54
Solution
As the link is constructed on teh BE and returned to the FE, this just displays the link in the correct place
Tests
verified by @kishore03109