Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Initial version of date column #1363
Initial version of date column #1363
Changes from 3 commits
cb6c192
3b85c38
b0e9468
7eef905
ebb47c8
0814bb0
ce913d3
5132ae3
c358842
5ef390f
1884711
2794b7a
554c50e
74e578d
199d44c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is a buggy behavior worth overriding. Did you check from that perspective?
If the goal is just making tests easier it should be a part of the page object. I'm not sure there is a clear benefit in trying to override the details of the native date formatter to get the exact same rendered text across browsers. Is there a specific argument for that?
One argument against could be that many apps are likely to use the same formatter. So our date text will render similarly to most other web apps in Chrome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a bug--just a mostly trivial difference between how each browser formats the date. It seems more targeted to doing the replacement here than in the page object. If I do it in the page object, I either have to do this substitution for all text rendered in cells, or add an esoteric new function to fetch a rendered date from a cell. I don't see any practical downsides to replacing one narrow non-breaking space with a regular space. That was my thought process, at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the date table column could have it's own page object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit: removed, wasn't very productive