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

Remove hard coded HTML Ids from UserFields.Edit.cshtml to allow reusing it #12532

Merged
merged 4 commits into from
Oct 6, 2022

Conversation

MikeAlhayek
Copy link
Member

@MikeAlhayek MikeAlhayek commented Oct 1, 2022

Replace hard coded HTML ids with generated ID to allow the reuse of UserFields.Edit.cshtml

Fix #12531

@Skrypt
Copy link
Contributor

Skrypt commented Oct 3, 2022

Looks fine to me.

@hishamco
Copy link
Member

hishamco commented Oct 3, 2022

@Skrypt I didn't see such thing before in our code base, we already bound to the property name

@Skrypt
Copy link
Contributor

Skrypt commented Oct 3, 2022

No, it is not consistent with what we generally do, I agree. Though, a specific use case here seems to be fine to use. Though, he could have overridden the Razor template in its own Theme for that matter without breaking anything. So, it is debatable.

@MikeAlhayek
Copy link
Member Author

@hishamco yes may be this isn't consistent with what we generally do. IMO, since a view is called in a driver, it should be reusable so hard coding HTML ids is not a good idea as I learned :)

@Skrypt yes I can override this view in my own theme "this is what I do today as a workaround". But I thought, we should also make this view robust in OC since it's called from drivers.

Either way, I change the code in the view to make it more robust by removing the hard coded HTML ids completely, and use css classes. This way the Javascript code actually is called with no problem for every instance rendered on the HTML page.

@MikeAlhayek MikeAlhayek changed the title Replace hard coded HTML ids with generated ID Remove hard coded HTML Ids from UserFields.Edit.cshtml to allow reusing it Oct 3, 2022
@hishamco
Copy link
Member

hishamco commented Oct 5, 2022

Though, he could have overridden the Razor template in its own Theme for that matter without breaking anything

That's what supposed to be done, I diagree for adding things that's help user needs, where we provide an extensibility point for that

Let us hear the feedback from others

@Skrypt Skrypt merged commit 88d709e into OrchardCMS:main Oct 6, 2022
@MikeAlhayek MikeAlhayek deleted the AddUniqueUserFields branch December 28, 2022 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove hard coded HTML ids from UserFields.Edit.cshtml view
4 participants