Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add jump to date functionality to date headers in timeline v2 #7339

Merged

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Dec 11, 2021

v1 in #7317


Add jump to date functionality to date headers in timeline

Relies on this matrix-js-sdk PR to add MSC3030 /timestamp_to_event support: matrix-org/matrix-js-sdk#2072

Fix element-hq/element-web#7677

Part of MSC3030: matrix-org/matrix-spec-proposals#3030

Experimental Synapse implementation added in matrix-org/synapse#9445

Testing strategy

  1. Update your Synapse homeserver.yaml to enable the msc3030_enabled experimental feature
    experimental_features:
      msc3030_enabled: true
    
  2. Refresh Element so the Element Labs feature shows up
  3. Enable the Jump to date Labs feature
  4. Check out the timeline for the jump to date headers

Dev notes

Screen recordings:


Combining React callback refs and ref-refs,

const attachRef = (input) => {
    this.input = input;

    if (typeof this.props.inputRef === 'function') {
        this.props.inputRef(input);
    } else if(this.props.inputRef) {
        this.props.inputRef.current = input;
    }
}

TODO


This PR currently has no changelog labels, so will not be included in changelogs.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Preview: https://61f2350539af3b54bfe30e9a--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

t3chguy and others added 12 commits December 10, 2021 15:15
New TooltipTarget & TextWithTooltip were not roving-accessible
Copy changes from #7317
on top of RovingTabIndex fixes in #7336
 1. Only can tab to the datepicker input and "Go" when the "Jump to date" option is selected
 1. Tabbing off the end of the ContextMenu will close it and return your focus back to where
    it is on the page. Note: nothing special coded for this, just prevented from tabbing off
    the end of the ContextMenu which was the end of the page.
 1. Can navigate up and down the ContextMenu with Arrow keys as expected
@MadLittleMods MadLittleMods requested a review from a team as a code owner December 11, 2021 13:11
@MadLittleMods MadLittleMods removed the request for review from a team December 11, 2021 13:11
@MadLittleMods MadLittleMods marked this pull request as draft December 11, 2021 13:11
@ShadowJonathan
Copy link

Separate PR: Stick date headers so one is always visible to reference and available to use to jump, element-hq/element-web#7575

❤️

See #7339 (comment)

We also added `:not(.mx_AccessibleButton_hasKind)` to the `res/css/views/context_menus/_IconizedContextMenu.scss`
selectors so the style that was overriding this before is no longer interferring.
…s-in-timeline-v2

Conflicts:
	src/i18n/strings/en_EN.json
onDatePicked?: (dateString: string) => void;
}

const JumpToDatePicker: React.FC<IProps> = ({ ts, onDatePicked }: IProps) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review pass @turt2live 🙂

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

lgtm minus component naming

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

thanks :)

@MadLittleMods MadLittleMods merged commit 7fa27f5 into develop Jan 27, 2022
@MadLittleMods MadLittleMods deleted the madlittlemods/msc3030-jump-to-date-headers-in-timeline-v2 branch January 27, 2022 22:32
@MadLittleMods
Copy link
Contributor Author

Thanks for the review @turt2live 🐇

@turt2live
Copy link
Member

@MadLittleMods did this still need design review?

@MadLittleMods
Copy link
Contributor Author

@turt2live It's using the standard context menu styles so it's probably pretty good to go. The only thing non-standard is the jump to date calendar form area which we can always iterate in another PR if needed. Plus this is also behind a Labs flag so it doesn't really need to be held up by anything.

@t3chguy
Copy link
Member

t3chguy commented Jan 31, 2022

@MadLittleMods this caused all context menus to increase in size so definitely should've had design review

element-hq/element-web#20801

@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Jan 31, 2022

@t3chguy Thanks for cross-linking. It doesn't seem like we need to rely on design to QA that kind of thing (at least as the only defense). It can be spotted by anyone trying out the branch but we just missed it. Easy regression we can fix up on nightly ⏩ #7684

MadLittleMods added a commit that referenced this pull request Feb 1, 2022
Fix element-hq/element-web#20801

Regressed in #7339

Relevant styles were first added in #4858
(context behind why the original styles were added)
MadLittleMods added a commit that referenced this pull request Feb 1, 2022
Fix element-hq/element-web#20801

Regressed in #7339

Relevant styles were first added in #4858
(context behind why the original styles were added)

---

## Cause

Battling CSS specificity between the default and compact styles, https://specificity.keegan.st/

Known good (On `app.element.io` (expected)):
```css
// 0 3 0
.mx_IconizedContextMenu .mx_IconizedContextMenu_optionList .mx_AccessibleButton {
    padding-top: 12px;
    padding-bottom: 12px;
}

// Compact styles override our default rules because they come
// after the other styles (source order) and have the same specificity
// 0 3 0
.mx_IconizedContextMenu.mx_IconizedContextMenu_compact .mx_IconizedContextMenu_optionList > * {
    padding: 8px 16px 8px 11px;
}
```

Bad (On `develop` (broken)):
```css
// Default rules always override because they have higher specificity.
// The `:not()` selector doesn't add any extra specificity but the selectors inside the `:not(...)` do.
// 0 4 0
.mx_IconizedContextMenu .mx_IconizedContextMenu_optionList .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
    padding-top: 12px;
    padding-bottom: 12px;
}

// 0 3 0
.mx_IconizedContextMenu.mx_IconizedContextMenu_compact .mx_IconizedContextMenu_optionList > * {
    padding: 8px 16px 8px 11px;
}
```
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URL for an arbitrary day of history and navigation for next and previous days
4 participants