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

allow view/edit in new tabs from list view #22888

Merged
merged 5 commits into from
Oct 28, 2023

Conversation

nitram84
Copy link
Contributor

Fix #22801

It is interesting to note, that tabs were already supported by react, so there was nothing to do for react. With this PR the view and edit "buttons" for angular and vue (here only authentication type "session") work in the same way like react. Vue produces an error for links opened in new tabs when authentication type is not "session". Working with tabs works best in session mode, e.g. with JWT authentication a login for each tab is required. But even when a login is required, this PR makes it still easier to copy or share view/edit links.


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@deepu105
Copy link
Member

Can you check the failing tests plz. Probably snapshots need to be updated using the npm run update-snapshots command

@mshima mshima closed this Aug 12, 2023
@mshima mshima reopened this Aug 12, 2023
@nitram84
Copy link
Contributor Author

@deepu105: The failing test cases weren’t mine. At the time I created my PR all builds had failing test cases (and those were accepted). The build of the previous PR of my PR (https://github.com/jhipster/generator-jhipster/pull/22886/checks) had the same failing test cases as my PR. So these test failures weren't caused by my PR. It seems that the failing tests are now fixed on main, so my PR shouldn't pose a problem anymore.

DanielFran
DanielFran previously approved these changes Aug 20, 2023
@DanielFran
Copy link
Member

@nitram84 it is missing only react. Can you take a look?

@nitram84
Copy link
Contributor Author

@DanielFran I didn't miss react. In react new tabs are supported by default for view and edit:

"It is interesting to note, that tabs were already supported by react, so there was nothing to do for react. With this PR the view and edit "buttons" for angular and vue (here only authentication type "session") work in the same way like react. [...]"

This PR reduces the differences between the supported UI libs.

For react there might be one point: React supports delete operations in new tabs, so users can copy or share a delete-link. For view and edit links this is ok - those links can be used as permalinks. This is not true for delete links, so perhaps react should use a button for delete instead of a link, if this behavior is unwanted.

mraible
mraible previously approved these changes Aug 21, 2023
@DanielFran
Copy link
Member

DanielFran commented Aug 21, 2023

@nitram84 I agree, for delete it should be a button.
Do you want to include those changes in this pr?

@nitram84 nitram84 dismissed stale reviews from mraible and DanielFran via 0723d9c August 23, 2023 13:22
@nitram84
Copy link
Contributor Author

I fixed the react part too. Now all changes should be consistent.

DanielFran
DanielFran previously approved these changes Aug 23, 2023
@DanielFran
Copy link
Member

@nitram84 Can you fix the conflicts please?

@nitram84
Copy link
Contributor Author

@DanielFran Thank you for reminding me. I had to rebase my changes. Rebasing worked, all changes are identical to previously reviewed commits.

DanielFran
DanielFran previously approved these changes Oct 26, 2023
@DanielFran
Copy link
Member

@nitram84 Can you take a look at the e2e tests, some are failing

@DanielFran
Copy link
Member

Thanks @nitram84 for the contribution

@DanielFran DanielFran merged commit 98ae48b into jhipster:main Oct 28, 2023
47 checks passed
@deepu105 deepu105 added this to the 8.0.0 milestone Nov 2, 2023
@mshima
Copy link
Member

mshima commented Nov 12, 2023

This PR created a regression at react. See #24115.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UX/angular] Allow view/edit in new tabs from list view
5 participants