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

Default View Font Color and Reordering #6114

Closed
Bonapara opened this issue Jul 3, 2024 · 35 comments · Fixed by #6326
Closed

Default View Font Color and Reordering #6114

Bonapara opened this issue Jul 3, 2024 · 35 comments · Fixed by #6326
Labels
good first issue Good for newcomers scope: front Issues that are affecting the frontend side only size: short type: design improvement

Comments

@Bonapara
Copy link
Member

Bonapara commented Jul 3, 2024

Current Behavior

The default view font color is tertiary.
We cannot reorder the default view.

CleanShot 2024-07-03 at 17 40 30@2x

Desired Behavior

The text color should be primary, like other views' title color.
It should be possible to reorder this view.

CleanShot 2024-07-03 at 17 38 12@2x

Ensure this edit doesn't change the color of the non-movable item in the "Options > Fields" menu.

CleanShot 2024-07-03 at 17 39 19@2x

@Bonapara Bonapara added good first issue Good for newcomers scope: front Issues that are affecting the frontend side only size: minutes labels Jul 3, 2024
Copy link
Contributor

greptile-apps bot commented Jul 3, 2024

To address the issue:

  1. Change Default View Font Color:

    • Update the NavigationDrawerItem component in /packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx.
    • Modify the StyledItem styled component to set the default font color to primary.
    const StyledItem = styled('div', {
      shouldForwardProp: (prop) =>
        !['active', 'danger', 'soon'].includes(prop) && isPropValid(prop),
    })<StyledItemProps>`
      color: ${(props) => {
        if (props.active === true) {
          return props.theme.font.color.primary;
        }
        if (props.danger === true) {
          return props.theme.color.red;
        }
        if (props.soon === true) {
          return props.theme.font.color.light;
        }
        return props.theme.font.color.primary; // Change from secondary to primary
      }};
    `;
  2. Enable Reordering of Default View:

    • Ensure the RecordTableHeaderCell component in /packages/twenty-front/src/modules/object-record/record-table/components/RecordTableHeaderCell.tsx allows reordering.
    • Verify that the disableColumnResize logic does not prevent reordering for the default view.
    const disableColumnResize =
      column.isLabelIdentifier && isMobile && scrollLeft > 0;
    • Adjust the condition if necessary to allow reordering.

Ensure these changes do not affect the non-movable item in the "Options > Fields" menu.

References

/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx
/packages/twenty-front/src/modules/ui/display/typography/components/H1Title.tsx
/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableHeaderCell.tsx
/packages/twenty-front/src/modules/ui/theme/constants/FontLight.ts

About Greptile

This response provides a starting point for your research, not a precise solution.

Help us improve! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

Ask Greptile · Edit Issue Bot Settings

@adithej
Copy link
Contributor

adithej commented Jul 4, 2024

Hi @Bonapara

Is this the desired behavior you are looking for? Or are you expecting the default view eg: All Companies to be draggable as well?

Text color issue has been rectified

twenty-issue.mov

@Bonapara
Copy link
Member Author

Bonapara commented Jul 4, 2024

Hi @adithej, thanks for your work! I was indeed expecting the default view to be draggable too.

@adithej
Copy link
Contributor

adithej commented Jul 4, 2024

Thanks @Bonapara

I am working on it but not that sure I will be able to, but still being optimistic.
The default gets draggable but it goes back to the last position. I am not being able to find the root cause.

On ViewPickerListContent.tsx I have made these changes.

<DropdownMenuItemsContainer>
        <DraggableList
          onDragEnd={handleDragEnd}
          draggableItems={viewsOnCurrentObject.map(
            (view, index) => (
              <DraggableItem
                key={view.id}
                draggableId={view.id}
                index={index}
                isDragDisabled={viewsOnCurrentObject.length === 1}
                itemComponent={
                  <MenuItemDraggable
                    key={view.id}
                    iconButtons={indexView?.id === view.id ? 
                      [{
                        Icon: IconLock,
                      },]
                      :
                      [{
                        Icon: IconPencil,
                        onClick: (event: MouseEvent<HTMLButtonElement>) =>
                          handleEditViewButtonClick(event, view.id),
                      },]
                    .filter(isDefined)}
                    isIconDisplayedOnHoverOnly = {indexView?.id === view.id ? false : true}
                    onClick={() => handleViewSelect(view.id)}
                    LeftIcon={getIcon(view.icon)}
                    text={view.name}
                  />
                }
              />
            ),
          )}
        />
      </DropdownMenuItemsContainer>

And the handleDragEnd seems okay but I belive the state is not getting upaded.

const handleDragEnd = useCallback(
    (result: DropResult) => {
      if (!result.destination) return;
      
      moveArrayItem(viewsOnCurrentObject, {
        fromIndex: result.source.index,
        toIndex: result.destination.index,
      }).forEach((view, index) => {
        if (view.position !== index) {
          updateView({ ...view, position: index });
        }
      });
      // console.log("view before", viewsOnCurrentObject)
    },
    [updateView, viewsOnCurrentObject],
  );

It would be helpful if I could find any leads that may help me fix this

twenty-issue-2.mov

@Bonapara
Copy link
Member Author

Bonapara commented Jul 4, 2024

Perhaps Lucas Bordeaux has some thoughts for you!

@adithej
Copy link
Contributor

adithej commented Jul 5, 2024

Thanks @Bonapara and @lucasbordeau if you have some thoughts about it , will be wonderful.

@Faisal-imtiyaz123
Copy link
Contributor

@adithej May you tell me if you are still working here?

@adithej
Copy link
Contributor

adithej commented Jul 14, 2024

No @Faisal-imtiyaz123 if you are interested please have a look

@Faisal-imtiyaz123
Copy link
Contributor

@adithej Thanks for replying. Sure, I will take it up then

@Bonapara
Copy link
Member Author

It was not implemented, or there was a regression. We should try to understand what happened @charlesBochet

@charlesBochet
Copy link
Member

/oss.gg 300

Copy link

oss-gg bot commented Oct 9, 2024

Thanks for opening an issue! It's live on oss.gg!

@nikhiludayy
Copy link

/assign

Copy link

oss-gg bot commented Oct 12, 2024

@nikhiludayy, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

@nikhiludayy nikhiludayy removed their assignment Oct 12, 2024
@nikhiludayy
Copy link

/assign

Copy link

oss-gg bot commented Oct 12, 2024

Assigned to @nikhiludayy! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

Copy link

oss-gg bot commented Oct 13, 2024

@nikhiludayy, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

1 similar comment
Copy link

oss-gg bot commented Oct 14, 2024

@nikhiludayy, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

@charlesBochet
Copy link
Member

/unassign

Copy link

oss-gg bot commented Oct 14, 2024

Issue unassigned.

@vamus092
Copy link

/assign

Copy link

oss-gg bot commented Oct 14, 2024

Assigned to @vamus092! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

Copy link

oss-gg bot commented Oct 15, 2024

@vamus092, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

Copy link

oss-gg bot commented Oct 15, 2024

@nikhiludayy, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

1 similar comment
Copy link

oss-gg bot commented Oct 16, 2024

@nikhiludayy, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

@Bonapara
Copy link
Member Author

@vamus092 do you think you will be able to open a PR today?

@vamus092
Copy link

/unassign

Copy link

oss-gg bot commented Oct 22, 2024

Issue unassigned.

@oss-gg oss-gg bot unassigned vamus092 Oct 22, 2024
@nganphan123
Copy link
Contributor

/assign

Copy link

oss-gg bot commented Oct 22, 2024

Assigned to @nganphan123! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

Weiko pushed a commit that referenced this issue Oct 23, 2024
This PR fixes issue #6114 
I removed the separate check for default item and add it into the
draggable list.
@Weiko Weiko closed this as completed Oct 23, 2024
Copy link

oss-gg bot commented Oct 26, 2024

@nganphan123, please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12 hours, you will be unassigned automatically.

Copy link

oss-gg bot commented Oct 27, 2024

@nganphan123 has not opened a PR for this issue within 48 hours. They have been unassigned from the issue; anyone can now take it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers scope: front Issues that are affecting the frontend side only size: short type: design improvement
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

9 participants