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

Showed both row and column level context menu options in the cell level #2728 #2803

Merged
merged 16 commits into from Apr 17, 2023

Conversation

TafreedAhmad
Copy link
Contributor

@TafreedAhmad TafreedAhmad commented Apr 10, 2023

Fixes #2702

Technical details

The options that were previously available only in the row-level and column-level menus are now also present in the cell-level menu, and I have implemented their functionality to match that of the row-level and column-level menus.

Screenshots

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the develop branch of the repository
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no
    visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@TafreedAhmad TafreedAhmad changed the title made my branch back to as it was before Showed both row and column level context menu options in the cell level #2728 Apr 10, 2023
@pavish pavish self-assigned this Apr 10, 2023
@pavish pavish self-requested a review April 10, 2023 16:40
@pavish pavish added the pr-status: review A PR awaiting review label Apr 10, 2023
@pavish pavish assigned TafreedAhmad and unassigned pavish Apr 10, 2023
@pavish pavish added pr-status: revision A PR awaiting follow-up work from its author after review and removed pr-status: review A PR awaiting review labels Apr 10, 2023
@pavish
Copy link
Member

pavish commented Apr 10, 2023

@TafreedAhmad Make sure to fix formatting, and linting issues before marking the PR for review again.

@TafreedAhmad
Copy link
Contributor Author

@TafreedAhmad Make sure to fix formatting, and linting issues before marking the PR for review again.

@pavish You may check it now, I was just making my new branch equal to the old branch in the previous commit because my last fork was corrupted somehow so I had to re-fork and re-build it.

@dmos62 dmos62 requested a review from pavish April 11, 2023 09:32
@TafreedAhmad
Copy link
Contributor Author

@pavish It's done using props this time...

@rajatvijay
Copy link
Contributor

@TafreedAhmad the lint checks are still failing.

@rajatvijay rajatvijay added this to the Backlog milestone Apr 12, 2023
@TafreedAhmad
Copy link
Contributor Author

@rajatvijay I've updated it but the lint checks aren't running now

@TafreedAhmad
Copy link
Contributor Author

@pavish I am unable to run lint tests locally due to some errors but I have removed them and have formatted the document several times and it is still giving me errors...

@pavish
Copy link
Member

pavish commented Apr 13, 2023

@TafreedAhmad The errors on the workflow seem to be due to formatting issues.

You can run docker exec -it -w /code/mathesar_ui mathesar_service_dev npm run check-format to identify the files with the formatting errors.

You can then fix the errors manually or automatically using:
docker exec -it -w /code/mathesar_ui mathesar_service_dev npm run format

You can refer Prettier docs to better understand how formatting using Prettier works.

Refer our frontend readme file to understand how to perform formatting, linting, and testing.

@TafreedAhmad
Copy link
Contributor Author

TafreedAhmad commented Apr 13, 2023

@pavish Thanks a lot. I was really struggling to find the error here... I think it's done now.

@pavish pavish assigned pavish and unassigned TafreedAhmad Apr 14, 2023
@pavish pavish added pr-status: review A PR awaiting review and removed pr-status: revision A PR awaiting follow-up work from its author after review labels Apr 14, 2023
Copy link
Member

@pavish pavish left a comment

Choose a reason for hiding this comment

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

@TafreedAhmad Looks good! Thanks for your work on this.

I've added a few commits to clear things up:

  • 4e1af61 This commit directly passes down rowKey instead of the primaryKeyColumnId since we already calculate it in the parent component. It also removes the unnecessary String(rowKey) case, since rowKey is explicitly a string type.
  • 9b78059 This moves the 'Go To Linked Record' options to it's appropriate section, the cell level options section.
  • 06a69c8 This renames recordId to recordPk in RowContextOptions to better improve readability of the purpose of the prop.

@pavish pavish added this pull request to the merge queue Apr 17, 2023
Merged via the queue into mathesar-foundation:develop with commit 795a629 Apr 17, 2023
6 checks passed
@seancolsen
Copy link
Contributor

I opened #2846 as a small follow-up to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-status: review A PR awaiting review
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Show both row level and column level context menu options in the cell level context menu
4 participants