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

Implement basic form control support #3520

Merged
merged 8 commits into from Oct 1, 2014
Merged

Conversation

jdm
Copy link
Member

@jdm jdm commented Sep 29, 2014

So far the changes to layout seem fairly well-contained; I think this is worth integrating to give us a browser that is easier to dogfood (and allows us to work on things like form submission much easier), especially since the long-term viability of WebComponents-as-forms is not assured.

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/2724

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

mem::transmute::<&RefCell<Option<String>>, &Option<String>>(&(*input.unsafe_get()).value).clone()
}

match (*self.unsafe_get()).input_type.get() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use generated content in the UA stylesheet here instead? input[checked="checked"] { content: ... }

@jdm
Copy link
Member Author

jdm commented Sep 29, 2014

I've switched to generated content; the disadvantage is that it can now be split whereas previously I don't think it could. At least that's the difference I see on http://tim.dreamwidth.org/ (look at the Remember Me checkbox).

pub enum InputFragmentInfo {
InputButton(u32),
InputText(u32),
InputCheckbox,
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like most of these are strictly needed at the moment. Can we use GenericFragment instead for InputCheckbox and InputRadioButton and collapse the InputButton/InputText/InputFile down into one?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are correct.

@pcwalton
Copy link
Contributor

pcwalton commented Oct 1, 2014

r=me for the layout parts, it's nice and minimal now. Thanks! 👍

@jdm
Copy link
Member Author

jdm commented Oct 1, 2014

@Manishearth Feel like reviewing the DOM stuff?

@Manishearth
Copy link
Member

Done.

bors-servo pushed a commit that referenced this pull request Oct 1, 2014
So far the changes to layout seem fairly well-contained; I think this is worth integrating to give us a browser that is easier to dogfood (and allows us to work on things like form submission much easier), especially since the long-term viability of WebComponents-as-forms is not assured.
bors-servo pushed a commit that referenced this pull request Oct 1, 2014
So far the changes to layout seem fairly well-contained; I think this is worth integrating to give us a browser that is easier to dogfood (and allows us to work on things like form submission much easier), especially since the long-term viability of WebComponents-as-forms is not assured.
@bors-servo bors-servo closed this Oct 1, 2014
@bors-servo bors-servo merged commit be41c20 into servo:master Oct 1, 2014
@jdm jdm deleted the formcontrols branch August 4, 2015 04:28
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.

None yet

6 participants